Terra-core: Tag Component

Created on 19 Feb 2018  路  44Comments  路  Source: cerner/terra-core

Issue Description

Create a new Tag component with OCS theming.

Issue Type

  • [X] New Feature
  • [ ] Enhancement
  • [ ] Bug
  • [ ] Other
terra-tag

Most helpful comment

I would think the icon and the text together needs to be clickable.

All 44 comments

Technical Design(Revised): terra-tag

Summary:

This is a new component that allows the consumer to display a tag.

Expected behavior:

Should be able to give the component an icon and a text.

Responsiveness:

The content within the tag does not change. The width of the tag is dictated by the length of the content. On content wrap the icons will vertically center within the tag container.

I18N / Bi-directionality:

This component must be able to be rendered with its mirror-imaged equivalent for rtl locales.

Accessibility

The text is accessible on basic tag.

Interaction States

Unselected, Selected, Hover and Keyboard focus will be available for this tag.

React Props:

| Props | Type | Default | Required |Description |
| ----------- | ----- | --------- | ---------- |--------------------------------------------------------- |
| href | string | undefined | No | Sets the href. When set will render the component as an anchor tag. |
| icon | element | undefined | No | The icon to render. |
| text | string | `| Yes | Sets the text of the tag. | |onClick| function | undefined | No | Callback function triggered when clicked. | |theme` | string | 'normal' | No | The theme to be applied to the tag.

Themeable Styles:

  • Background color
  • Border
  • Font size
  • Font weight
  • Padding
  • Height
  • Box-Shadow
  • Theme

Can you rename 'isClose' to 'isClosed'?

@ChaseSinclair Updated it.

From looking at the callouts, it appears that 'text' should be a required prop.

Added a column 'Required' .

+1

Actually, would 'isClosable' be a more appropriate name since the prop is really indicating if you can click the X on the tag?

@paku-sruti For the close prop, let's call it onClose. This aligns with how we've handled this use case in the action header. The onClose prop should take a callback function for when the close button is clicked. If this is set, the close icon will display.

  1. The close button I think needs to be themable. Please double check with Chris.
  2. Would the text inside the tag wrap or it needs to show the entire text passed in? Does there need to be some truncation rules?
  3. Add accessibility for tags, when we tab it needs to first access the icon and the text part and on tabbing second it needs to focus on the close icon(if present).

@dm123455 According to the callouts it says the tag will adjust width to fit the content. The text should not truncate.

Do we want the icon to be clickable too or just the text needs to be clickable?

@neilpfeiffer Do we want the Close icon in the tag component to be themable?

+1 on @dm123455 's comment about the close icon needing to be themeable.

@paku-sruti I believe that the configurable (non-closing) icon is supposed to remain centered vertically when text wraps, as it doesn't truncate, according to the callouts

Looking at the callouts for cerner-consumer-theme, it also looks like you need the ability to do both a light and dark theme. I would check with @dm123455 about this, I know we originally ran into the same issue with buttons but I think we scoped it out. We might want to do the same thing here, but if we aren't scoping it out, we'll need to provide a way of handling that in this tech-design.

I would think the icon and the text together needs to be clickable.

Also @paku-sruti this example might help with the onClose implementation part.

@paku-sruti I know in standard html, we have a tabindex option to control tabbing order, and react has its own concept of this, but I don't know how this is handled when a component specifying either of these properties is placed in the context of a page with other components that may or may not have the same props specified. This might require a little bit of a spike to figure out. I would suggest googling / looking through terra for examples.

For the theme-able property font-family, I think we should confirm this as I remember on the buttons story Emily had mentioned to me that we do not want to be setting fonts on component level and they should rather be inherited from terra-base.

I might set the default value of the text prop as '', not undefined.

@samvaity @paku-sruti so I heard from @neilpfeiffer yesterday, font-family is NOT something that's supposed to be controllable at this level, UX designers have been throwing it on callouts due to an ambiguity in dev's vs designer's ideas of what's themeable. font-family should always be set to inherit!

@bjankord Updated the onClose to a call back function.

@dylan-sftwr and @samvaity Removed the font family from themable variables.

@dm123455 Added the accessibility and added the details about the text wrap in the responsiveness.

@dm123455 i'm wondering if we should have an aria-label or something similar for the text for our friends with screen readers?

Also, is there a use-case in which we have any other border-style, so that it should be theme-able ?

@samvaity it's mentioned in the callouts

@dylan-sftwr that way even font-family is mentioned....

  1. Recap of some of the previous questions:
  2. Close icon to be themeable: YES
  3. Icon and the text together needs to be clickable: YES
    --> full left "tab" and background is the full target, not just the the text
  4. do not want to be setting fonts on component level: CORRECT - do not include font-family
    --> font-family inherited from base
  5. light and dark theme: Light/Normal theme only for initial release
  6. Border style / Border color: Collapse into single Border variable
    --> Terra default theme will remove border and only use background

\
\

  1. I would update the language in the Accessibility section, or think about it in this way:

The text is accessible on basic tag.
If icon and close icon are present the first tab will access the icon and text and the second tab will access the close icon.
The first tab will access the optional icon and text; if the component is provided a method to close, the second tab will access the close icon.

Add box-shadow as themeable variable, needed for focus styles
(terra default theme will also need, page is missing so I will provide and update Zeplin)

Updated the text in accessibility, added box shadow in themable variables, added a prop to set the theme of the tag, added close icon to be themable.

After a conversation with Neil, I think we're going to want this to support bidirectional functionality, i.e. the close icon would be able to be on the left and the configurable icon on the right in a rtl locale

@paku-sruti light and normal theme not dark for the theme prop.

Also, Since this component is very similar to the button component.

When considering theme-ing and since we have the background color to be theme-able, shouldn't the color of the text also be theme-able?

@samvaity In the callouts the font-color/text-color is mentioned under the Styles:Controlled.

+1

nit: change the text in the Default column of the props table for string types from the backtick to a '.

@dm123455 , with light and normal, for the theme prop, are you describing the tag, or the context in which they'd be getting used? normal to me sounds like you're describing the tag appearance, rather than the context in which it would be getting used.

@paku-sruti If that's the case, I think the Default value of the theme prop would be normal, not light.

@paku-sruti & @dm123455, if I'm able to be confused about this, I'm sure others might too... rather than ('light' || 'normal'), could we do something like ( ( 'default' || 'normal') || 'inverse') ? I think doing so better describes the appearance of the tags without the ambiguity.

@dm123455 I know we're only doing the darker appearance of the tags for right now, but have we thought about how the configurable icon should be passed? Meaning, is it up to the user to ensure that their usage of the theme prop and their supplied icon prop appear appropriately together? Or is there some sort of styling to the icon that we'll have to do under the hood as a part of this component?

@dylan-sftwr As we spoke offline updated the theme to accept normal value.

+1

+1

+1

+1 on tech design

JIRA created

Was this page helpful?
0 / 5 - 0 ratings

Related issues

noahbenham picture noahbenham  路  5Comments

StephenEsser picture StephenEsser  路  5Comments

juliacalandro picture juliacalandro  路  3Comments

StephenEsser picture StephenEsser  路  4Comments

noahbenham picture noahbenham  路  5Comments