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

Jump to bottom

feat: use custom states for button and anchor button variants #31718

Merged
merged 10 commits into from
Jun 17, 2024
Merged

feat: use custom states for button and anchor button variants #31718

merged 10 commits into from
Jun 17, 2024

Conversation

chrisdholt
Copy link
Member

Previous Behavior

All styling hooked off attribute selectors.

New Behavior

All variants now get set using custom states via ElementInternals. In the case that states are not supported, an attribute with will be added to manage those scenario. So, for primary buttons, if states are not supported, state--primary will be added.

This gives long term benefits of being able to quickly remove the attribute fallback as custom state support grows in our matrix. It also has performance parity overall when including the attributes, making way for performance wins once that can be removed.

Related Issue(s)

  • Fixes #

@chrisdholt chrisdholt requested a review from a team as a code owner June 14, 2024 18:10
@fabricteam
Copy link
Collaborator

fabricteam commented Jun 14, 2024 •
edited
Loading

📊 Bundle size report

✅ No changes found

@chrisdholt chrisdholt force-pushed the users/chhol/update-button-and-anchor-button-custom-states branch from 3e187c3 to d8d0612 Compare June 14, 2024 20:16
@chrisdholt
Copy link
Member Author

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 4 pipeline(s).

davatron5000
davatron5000 approved these changes Jun 14, 2024
View reviewed changes
Copy link
Contributor

@davatron5000 davatron5000 left a comment

Choose a reason for hiding this comment

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

LGTM, few questions but nothing blocking. Would probably like @radium-v's POV since he worked on buttons last and has done more element internals work than me.

packages/web-components/src/button/button.options.ts Show resolved Hide resolved
packages/web-components/src/button/button.styles.ts Show resolved Hide resolved
packages/web-components/src/button/button.ts Outdated Show resolved Hide resolved
packages/web-components/src/styles/states/index.ts Show resolved Hide resolved
@chrisdholt chrisdholt force-pushed the users/chhol/update-button-and-anchor-button-custom-states branch from 69746a2 to 58f4a6a Compare June 14, 2024 20:50
@radium-v
Copy link
Contributor

Ideally, hovering or focusing on the element should display the anchor link in the browser:
image

@chrisdholt
Copy link
Member Author

Ideally, hovering or focusing on the element should display the anchor link in the browser: image

Yeah, I'm not sure how we get around this without help from the browser. I don't assume they'd enable elements with a role of link to participate here, because they can't guarantee the security of the custom element and that there are no side effects. Best we can do is manage the hover scenario, but focus likely won't work.

@chrisdholt
Copy link
Member Author

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 4 pipeline(s).

@chrisdholt chrisdholt force-pushed the users/chhol/update-button-and-anchor-button-custom-states branch 5 times, most recently from 45609f3 to 5be3b98 Compare June 17, 2024 17:38
@chrisdholt chrisdholt force-pushed the users/chhol/update-button-and-anchor-button-custom-states branch from 5be3b98 to 43cf404 Compare June 17, 2024 17:47
@chrisdholt chrisdholt force-pushed the users/chhol/update-button-and-anchor-button-custom-states branch from 43cf404 to 863c207 Compare June 17, 2024 17:57
@chrisdholt chrisdholt merged commit 1fb910e into microsoft:master Jun 17, 2024
18 checks passed
@chrisdholt chrisdholt deleted the users/chhol/update-button-and-anchor-button-custom-states branch June 17, 2024 18:49
EdDaWord pushed a commit to EdDaWord/fluentui that referenced this pull request Jun 17, 2024
@chrisdholt @EdDaWord
feat: use custom states for button and anchor button variants (micros…
6d6e8b0 
…oft#31718)
EdDaWord pushed a commit to EdDaWord/fluentui that referenced this pull request Aug 14, 2024
@mltejera @EdDaWord
fix:react-nav-preview: High contrast issues (microsoft#31704)
e5ce9d1 
chore(motion): add bundle size fixtures (microsoft#31730)

fix(react-calendar-compat): fixed focus indicator cut off (microsoft#31596)

feat: use custom states for button and anchor button variants (microsoft#31718)

docs: add min-width styles to Table and DataGrid stories, add best practices (microsoft#31721)

fix:react-nav-preview: High contrast issues (microsoft#31704)

Pretty & lint & yarn change

Somehow missed change files

Fix one of the tests

Fix React-text test

Use hashed name for each semantic and control token
EdDaWord pushed a commit to EdDaWord/fluentui that referenced this pull request Aug 14, 2024
@mltejera @EdDaWord
fix:react-nav-preview: High contrast issues (microsoft#31704)
f32f216 
chore(motion): add bundle size fixtures (microsoft#31730)

fix(react-calendar-compat): fixed focus indicator cut off (microsoft#31596)

feat: use custom states for button and anchor button variants (microsoft#31718)

docs: add min-width styles to Table and DataGrid stories, add best practices (microsoft#31721)

fix:react-nav-preview: High contrast issues (microsoft#31704)

Pretty & lint & yarn change

Somehow missed change files

Fix one of the tests

Fix React-text test

Use hashed name for each semantic and control token
EdDaWord pushed a commit to EdDaWord/fluentui that referenced this pull request Aug 16, 2024
@chrisdholt @EdDaWord
feat: use custom states for button and anchor button variants (micros…
e84c98b 
…oft#31718)
EdDaWord added a commit that referenced this pull request Aug 19, 2024
@EdDaWord
Try a single file change
8a89eb6 
Change most tokens in useButtonStyles file

Give all the react-components a try

chore(motion): add bundle size fixtures (#31730)

fix(react-calendar-compat): fixed focus indicator cut off (#31596)

feat: use custom states for button and anchor button variants (#31718)

feat: update badge to use custom states (#31733)

fix: Teaching bubble carousel tabs are visible in windows high contrast (#31734)

fix:react-nav-preview: High contrast issues (#31704)

Pretty & lint & yarn change

Somehow missed change files

Fix one of the tests

Fix React-text test
EdDaWord added a commit that referenced this pull request Aug 19, 2024
@EdDaWord
Try a single file change
537f055 
Change most tokens in useButtonStyles file

Give all the react-components a try

fix: Teaching bubble carousel tabs are visible in windows high contrast (#31734)

fix:react-nav-preview: High contrast issues (#31704)

chore(motion): add bundle size fixtures (#31730)

fix(react-calendar-compat): fixed focus indicator cut off (#31596)

feat: use custom states for button and anchor button variants (#31718)

docs: add min-width styles to Table and DataGrid stories, add best practices (#31721)

fix:react-nav-preview: High contrast issues (#31704)

Pretty & lint & yarn change

Somehow missed change files

Fix one of the tests

Fix React-text test

Use hashed name for each semantic and control token

Build

missed one

Fix small mistake

Fix small mistake

Fix small mistake
EdDaWord added a commit that referenced this pull request Aug 19, 2024
@EdDaWord
Try a single file change
ffdafe1 
Change most tokens in useButtonStyles file

Give all the react-components a try

fix: Teaching bubble carousel tabs are visible in windows high contrast (#31734)

fix:react-nav-preview: High contrast issues (#31704)

chore(motion): add bundle size fixtures (#31730)

fix(react-calendar-compat): fixed focus indicator cut off (#31596)

feat: use custom states for button and anchor button variants (#31718)

docs: add min-width styles to Table and DataGrid stories, add best practices (#31721)

fix:react-nav-preview: High contrast issues (#31704)

Pretty & lint & yarn change

Somehow missed change files

Fix one of the tests

Fix React-text test

Use hashed name for each semantic and control token

Build

missed one

Fix small mistake

Fix small mistake

Fix small mistake

Fix Random YML File
EdDaWord added a commit that referenced this pull request Aug 20, 2024
@EdDaWord
Try a single file change
6bbdaed 
Change most tokens in useButtonStyles file

Give all the react-components a try

fix: Teaching bubble carousel tabs are visible in windows high contrast (#31734)

fix:react-nav-preview: High contrast issues (#31704)

chore(motion): add bundle size fixtures (#31730)

fix(react-calendar-compat): fixed focus indicator cut off (#31596)

feat: use custom states for button and anchor button variants (#31718)

docs: add min-width styles to Table and DataGrid stories, add best practices (#31721)

fix:react-nav-preview: High contrast issues (#31704)

Pretty & lint & yarn change

Somehow missed change files

Fix one of the tests

Fix React-text test

Use hashed name for each semantic and control token

Build

missed one

Fix small mistake

Fix small mistake

Fix small mistake

Fix Random YML File

Fix rebase mistake
EdDaWord added a commit that referenced this pull request Aug 23, 2024
@EdDaWord
Try a single file change
3a595f4 
Change most tokens in useButtonStyles file

Give all the react-components a try

fix: Teaching bubble carousel tabs are visible in windows high contrast (#31734)

fix:react-nav-preview: High contrast issues (#31704)

chore(motion): add bundle size fixtures (#31730)

fix(react-calendar-compat): fixed focus indicator cut off (#31596)

feat: use custom states for button and anchor button variants (#31718)

docs: add min-width styles to Table and DataGrid stories, add best practices (#31721)

fix:react-nav-preview: High contrast issues (#31704)

Pretty & lint & yarn change

Somehow missed change files

Fix one of the tests

Fix React-text test

Use hashed name for each semantic and control token

Build

missed one

Fix small mistake

Fix small mistake

Fix small mistake

Fix Random YML File

Fix rebase mistake

Fix mistake on number of quotes

Fix any possible merge conflicts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fabricteam fabricteam fabricteam left review comments

@janechu janechu janechu approved these changes

@radium-v radium-v radium-v approved these changes

@mlijanto mlijanto mlijanto approved these changes

@davatron5000 davatron5000 davatron5000 approved these changes

Assignees
No one assigned
Labels
web-components
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@chrisdholt @fabricteam @radium-v @davatron5000 @mlijanto @janechu