Tokens component #1485
Tokens component #1485
Conversation
Blackbaud-SteveBrush
requested review from
Blackbaud-JaminQuimby
and removed request for
Blackbaud-JaminQuimby
Codecov Report
@@ Coverage Diff @@
## master #1485 +/- ##
=======================================
Coverage 100% 100%
=======================================
Files 383 387 +4
Lines 7440 7596 +156
Branches 951 977 +26
=======================================
+ Hits 7440 7596 +156
Continue to review full report at Codecov.
|
Blackbaud-SteveBrush
requested review from
Blackbaud-JaminQuimby,
Blackbaud-BobbyEarl,
Blackbaud-PaulCrowder and
Blackbaud-ToddRoberts
| }) | ||
| export class TokensVisualComponent { | ||
| public colors: SkyToken[] = [ | ||
| { name: 'Red' }, |
There was a problem hiding this comment.
Just out of curiosity, why is red the only color that is not in alphabetical order? (It's the same in another file below.)
| </h3> | ||
|
|
||
| <p> | ||
| These tokens may be navigated with the arrow keys, and dismissed by either clicking the close button on each token, or pressing the backspace key. |
There was a problem hiding this comment.
I'd tweak this to: "These tokens can be navigated with the arrow keys, and they can be dismissed with the close buttons or the backspace key."
| </h3> | ||
|
|
||
| <p> | ||
| These tokens cannot be dismissed. However, they may still be navigated using the arrow keys. |
There was a problem hiding this comment.
I'd change this to: "These tokens cannot be dismissed, but they can still be navigated with the arrow keys."
| </h3> | ||
|
|
||
| <p> | ||
| These tokens define a custom property to display their value, and emit an event when a token is selected. |
There was a problem hiding this comment.
I'd split this into 2 sentences: "These tokens define a custom property to display their values. When users select a token, it emits an event."
| <p> | ||
| <button | ||
| (click)="focusLastToken()"> | ||
| Focus last token |
There was a problem hiding this comment.
I haven't served this up, so maybe it's clearer in context. But just looking at the code, I don't follow what this label means. Should there be an "on" between "Focus" and "last"?
| @@ -819,6 +819,10 @@ | |||
| "_description": "The close button for the timepicker modal", | |||
| "message": "Done" | |||
| }, | |||
| "token_dismiss_button_title": { | |||
| "_description": "The default text for the token dismiss button title.", | |||
There was a problem hiding this comment.
Probably doesn't matter too much, but "token dismiss button title" is a mouthful. I try to avoid long streams like this of nouns/verbs used as adjectives because it's hard to tell which words are modifiers and which word is being modified. I'd change "token dismiss button title" to "title of the button that dismisses tokens."
There was a problem hiding this comment.
This is more of an identifier than anything, and I believe the convention is to prefix the slug with the name of the module (can be read as: "Token > Dismiss button > Title").
There was a problem hiding this comment.
OK. Sorry about that.
This reverts commit 6511337.
The tokens component allows you to display a series of selected objects. Each token may be removed from the list with a close button or by clicking the token and pressing the escape key. More items may be added via the token stream. Tokens can be navigated with the left and right arrow keys.
View the VSTS build result: https://host.nxt.blackbaud.com/skyux2?_sp.spa=pr-tokens-component&component=SkyTokensDemoComponent
The filter component is now using the tokens: https://host.nxt.blackbaud.com/skyux2?_sp.spa=pr-tokens-component&component=SkyFilterDemoComponent