Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat: Create New Tag Theme #10100

Merged
merged 12 commits into from
May 25, 2023
Merged

Feat: Create New Tag Theme #10100

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
489715b
refactor(Tag): update component and theme for new DS
TylerAPfledderer Apr 15, 2023
5beccc3
Merge branch 'dev' into feat/new-tag-theme
TylerAPfledderer May 2, 2023
2fbbb64
fix(theme/tag): give variant objects `definePartsStyle`
TylerAPfledderer May 12, 2023
6ad9992
style(theme/tag): collapse lineHeight for label
TylerAPfledderer May 12, 2023
26f3db4
style(theme/tag): remove margin from close button
TylerAPfledderer May 12, 2023
e8b01f8
fix(tutorials): spread the remaining props for `FilterTag`
TylerAPfledderer May 12, 2023
b5d88aa
style(tutorials): shrink line height for the `FilterTag`
TylerAPfledderer May 12, 2023
3a9e5ff
refactor(Tag): create story for basic style variants
TylerAPfledderer May 17, 2023
fa5b0e2
refactor(theme/tag): update focus/hover styling for close button
TylerAPfledderer May 17, 2023
db32d09
Merge branch 'dev' into pr/TylerAPfledderer/10100
pettinarip May 17, 2023
2bdde99
refactor(theme): update colors
TylerAPfledderer May 18, 2023
18019aa
replace bodyLight with bodyMedium
pettinarip May 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 0 additions & 57 deletions src/@chakra-ui/gatsby-plugin/components/Tag.ts
View file
Open in desktop

This file was deleted.

123 changes: 123 additions & 0 deletions src/@chakra-ui/gatsby-plugin/components/Tag/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
import {
createMultiStyleConfigHelpers,
theme,
defineStyle,
} from "@chakra-ui/react"
import { tagAnatomy } from "@chakra-ui/anatomy"
import { $badgeColor, STATUS_COLORS } from "./utils"
import { defineMergeStyles } from "../components.utils"

const { definePartsStyle, defineMultiStyleConfig } =
createMultiStyleConfigHelpers(tagAnatomy.keys)

const { Tag: tagTheme } = theme.components

const baseStyleContainer = defineMergeStyles(tagTheme.baseStyle?.container, {
border: "1px",
borderColor: "transparent",
gap: 1,
borderRadius: "full",
px: 2,
minH: 8,
fontWeight: 300,
"&:any-link": {
textDecor: "none",
_focusWithin: {
outline: "4px solid",
outlineColor: "transparent",
outlineOffset: 0,
},
},
})

const baseStyleLabel = defineStyle({
...tagTheme.baseStyle?.label,
fontSize: "xs",
textTransform: "uppercase",
textAlign: "center",
TylerAPfledderer marked this conversation as resolved.
Show resolved Hide resolved
lineHeight: 1,
})

const baseStyleCloseButton = defineStyle({
...tagTheme.baseStyle?.closeButton,
opacity: 1,
TylerAPfledderer marked this conversation as resolved.
Show resolved Hide resolved
m: 0,
// Clear default
_focusVisible: null,
"&:focus-visible, &:hover": {
outline: "3px solid",
outlineOffset: "-2px",
},
Comment on lines +45 to +50
Copy link
Contributor Author

@TylerAPfledderer TylerAPfledderer May 17, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cheap solution to get focus/hover styling for the close button.

  1. override default styling by nulling out (because I'm using a multi-selector instead)
  2. Provide a more generous styling of an outline ring on hover or focus-visible (this is taking advantage of the default outlineColor value of currentColor)

Anything else preferred here?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! looks great now

})

const baseStyle = definePartsStyle({
container: baseStyleContainer,
label: baseStyleLabel,
closeButton: baseStyleCloseButton,
})

const getStatusStyles = (status: string, variant: string) => {
const statusStyles = STATUS_COLORS[status][variant]

return {
container: statusStyles,
}
}

const variantSubtle = definePartsStyle((props) => {
const { status = "normal" } = props
const defaultStyles = tagTheme.variants?.subtle(props)
const statusStyles = getStatusStyles(status, "subtle")
return {
container: {
...defaultStyles?.container,
// Remove default dark mode styles
_dark: {},
...statusStyles.container,
},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we also need to style the closeButtons pseudo classes with the same styles that the container has. I'm talking specifically about the focus state.

Right now we are displaying the default Chakra styles here. I think we should use the same border styles and colors that we use for the container.

This is how it looks now when you focus on the close button:
image

Not saying that this is the code but could be something like:

closeButton: {
  _focusVisible: {
    ...statusStyles["&:any-link"]._focusWithin,
    bg: "transparent",
    boxShadow: "none",
  },
},

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pettinarip This example you gave will actually work here.

I think what I am missing here is whether or not this button will actually be visible if the tag itself is a link. That might change how I approach this to ensure the focus ring for the button has enough contrast to the background of the tag (looking at the normal tag)

@nloureiro I don't believe we received that clarity in the DS yet. 😄

Will the close button ever be used if the tag is a link?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's a corner case, we might never have that.

For me, Tags must be prepared to be links, normally in the form of filters.
The close on the Tag is when you have a filter list and you want to remove that filter. (we currently don´t have this on ethereum.org)

if it's complex to have link + close has a possible variant I'm ok with don't adding this now and update when needed

does it help?

Copy link
Contributor Author

@TylerAPfledderer TylerAPfledderer May 15, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nloureiro Oh! I understand.

I think it will be best to have three stories here:

  1. Show all tag style variants as links with text and no icons to simply demonstrate the state changes (maybe they should include the + icon as the right-side element?).
  2. Provide another set of the same tags, but not as a links and containing the close button to demonstrate the button's styling in the context of the different style variants
  3. The different variants of elements that are expected to be rendered in the component.

Would this be better?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hum... I think it might work, as far as I understand.

correct me if I'm wrong:
we will have a folder in storybook with "tags" > inside there will be 3 stories

is that it?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct @nloureiro here you will see another entry
image

@TylerAPfledderer yes, make sense 👍🏼

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sweet! I'll get that added asap

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

look good to me, great job!!

}
})

const variantSolid = definePartsStyle((props) => {
const { status = "normal" } = props
const defaultStyles = tagTheme.variants?.solid(props)
const statusStyles = getStatusStyles(status, "solid")
return {
container: {
...defaultStyles?.container,
// Remove default dark mode styles
_dark: {},
...statusStyles.container,
},
}
})

const variantOutline = definePartsStyle((props) => {
const { status = "normal" } = props
const defaultStyles = tagTheme.variants?.outline(props)
const statusStyles = getStatusStyles(status, "outline")
return {
container: {
...defaultStyles?.container,
boxShadow: "none",
borderColor: $badgeColor.reference,
// Remove default dark mode styles
_dark: {},
...statusStyles.container,
},
}
})

const variants = {
subtle: variantSubtle,
solid: variantSolid,
outline: variantOutline,
}

export const Tag = defineMultiStyleConfig({
baseStyle,
variants,
defaultProps: {
variant: "subtle",
},
})
Loading