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.

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

Bordered Button #3325

Closed
Tracked by #3288
origami-z opened this issue Apr 26, 2024 · 17 comments · Fixed by #4055
Closed
Tracked by #3288

Bordered Button #3325

origami-z opened this issue Apr 26, 2024 · 17 comments · Fixed by #4055

Comments

@origami-z
Copy link
Contributor

origami-z commented Apr 26, 2024

Added bordered option to secondary button, for both current and theme next

Keep backwards compatibility (variant), add new prop

variant new type new color
cta filled accent
primary filled neutral
secondary transparent neutral

Initial design exploration: https://www.figma.com/design/DWClNw9QaYIRU9GzIdmOpQ/Salt-%26-MB-Theme-Palette-Unified-(r8)-alt-4?node-id=7526-49989&t=iSG2xTZ8enJXj5OH-1

@origami-z
Copy link
Contributor Author

Updated est. sprint

@pseys
Copy link
Contributor

pseys commented Jun 24, 2024

Nomenclature

Agreement to use the following terms for the new props appearance and color. The former will have the options solid, outline, and transparent. The latter will have accent and neutral.

variant new appearance new color
cta solid accent
outline accent
transparent accent
primary solid neutral
outline neutral
secondary transparent neutral

The default setting for Button will be appearance: solid, colour: neutral

Tokens/Variables

  • Agreement to deprecate the current "cat" | "primary" | "secondary" actionable tokens to be replaced with a more efficient reduced set of tokens.
    -- content-bold-foreground
    -- content-bold-foreground-disabled
    -- actionable-background
    -- actionable-background-hover
    -- actionable-background-active
    -- actionable-background-disabled
    -- actionable-background-readonly
    -- actionable-border
    -- actionable-border-disabled
    -- actionable-accent-background
    -- actionable-accent-background-hover
    -- actionable-accent-background-active
    -- actionable-accent-background-disabled
    -- actionable-accent-border
    -- actionable-accent-border-disabled
    -- actionable-subtle-background
    -- actionable-subtle-border

This token structure will be used and extended in #3326

@pseys
Copy link
Contributor

pseys commented Jun 24, 2024

  • Figma variables in the Salt (Next) Styles library have been updated and published

@pseys
Copy link
Contributor

pseys commented Jun 27, 2024

All instances of button have been updated in the Salt (Next) Components & Patterns library. The branch has been merged and is ready to publish.

@pseys pseys moved this from Planned to Green in Salt - Components, Patterns and Theming Jun 27, 2024
@pseys
Copy link
Contributor

pseys commented Jun 28, 2024

We've run into an issue where the foreground colour in legacy variables cta|primary|secondary don't meet the required contrast ratios for AA compliance. I've recommended an alternative solution that maps for both current and next – awaiting agreement from the team before proceeding further.

@origami-z
Copy link
Contributor Author

My feedback from token naming meeting:

  • I don't think we had an agreement on "restricting" our user to theme the components on characteristics layer
    • Our system principle has been (IMO), we made it easy for the users to do things and best practises via documentation, however, we try not to restrict user to do different things if necessary
    • Reducing number of tokens is the purpose of newly designed palette layer, so they can use minimal token to achieve maximum impact
    • On the other hand, characteristics existed to "fine tune" each variant of components / similar components
      The proposed change make a consumer hard to reason about how to customizing tokens (by reading the token names), without deep association with each component deesign / variants
  • Naming convention changed between actionable and other existing characteristics
    • some color / appearance uses state suffix approach ( actionable-subtle-background actionable-accent-background-hover ), some doesn't ( actionable-subtle-border vs actionable-accent-border)
    • foreground color flipping between content-primary-foreground content-bold-foreground, v.s. selectable-foreground selectable-foreground-hover
    • Proposed change makes it hard for others to follow token naming convention, to create tokens for any components

@origami-z
Copy link
Contributor Author

I'm happy to proceed with new token proposal to avoid delay, i still have concerns around foreground tokens

  • content-* switching between states means we're baking design decision of specific components to theme tokens, meaning that we'll lose the capability for users to customize them individually. This is a semi-breaking change for the commitment we're making towards "stable" characteristics last time we did the big theme token deprecation and update. We'll need to clearly communicate this.
  • some foreground using actionable-*, some using content , even this is a "compromise" for backwards compatibility, it will be hard to communicate how to do this. Personally i prefer using old naming convention across the board now, then reconcile when old button is completely gone. With above approach, we're setting ourselves us with outliner in naming convention for a while. That's more trouble than maintaining more tokens (which is used only by button and unlikely to change).

@bhoppers2008
Copy link

Currently blocked pending theme agreement @pseys @origami-z

@mark-tate
Copy link
Contributor

Goal: unblock and get to a known state

@mark-tate
Copy link
Contributor

Leads to review doc, re-group on Wednesday 10th July

@pseys
Copy link
Contributor

pseys commented Jul 19, 2024

Proposed token structures for Next and Legacy completed and agreed. Documentation provided for dev.

@mark-tate
Copy link
Contributor

Goal: feedback to Pepper and styles into Figma, requires work for both Legacy and Next
By EOS, close out

@origami-z
Copy link
Contributor Author

Will be released in #3880

@joshwooding
Copy link
Contributor

Aug 15 - Meeting tomorrow about this

@mark-tate
Copy link
Contributor

Will be released by 13th September

@origami-z
Copy link
Contributor Author

@pseys
Copy link
Contributor

pseys commented Sep 9, 2024

All supporting Figma files and libraries have been updated now and are ready to publish.

@origami-z origami-z self-assigned this Sep 13, 2024
@origami-z origami-z linked a pull request Sep 18, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment