-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
feat(button): add button to web components v3 #27278
feat(button): add button to web components v3 #27278
Conversation
📊 Bundle size report🤖 This report was generated against 3c926baf2bd55186e277b61652573f7aef85a80c |
Asset size changesUnable to find bundle size details for Baseline commit: 7f5a5e5 Possible causes
Recommendations
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 69b7f0e:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other than the callouts in the stylesheet this looks good. Maybe lets try running the css selector analyzer and see if there i an consolidation that could happen here.
d196df0
to
01d20ab
Compare
change/@fluentui-web-components-be1b97fa-2097-4adc-ad03-f84e0ac0b1b1.json
Outdated
Show resolved
Hide resolved
background-color: ${colorTransparentBackground}; | ||
} | ||
|
||
:host(:is([disabled], [disabled-focusable], [appearance][disabled], [appearance][disabled-focusable])) .control, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please explain why the appearance part of the selector ([appearance][disabled], [appearance][disabled-focusable]
) is necessary?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
specificity - I can try and pull it off, but we have a number of styles where we're changing the various button states themselves, many which use attribute selectors. This ensures that regardless of the actual attribute value, ALL appearances should have these styles applied in both disabled and disabled-focusable scenarios. It's possible that :is() provides enough specificity...but I'm not sure. I had to add :is()
in order to combine these AND get the right specificity (:where()
was not specific enough, by design). I can test when I get the repo built again.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should document Unfortunately, the correct default value is an empty string...which does reflect here. We could probably open an issue to see if we can adjust for that single scenario but nothing comes to mind currently. The only other delta (I think) we really have is |
7567615
to
29739fe
Compare
* add button as a new web component * change files * add package export * add icon attr, fixup disabled styles * update color => fill * add disabled focusable click handling * move to attribute syntax for aria-disabled * remove icon attribute in favor of min-height to accommodate icons * use long form for padding to ensure we catch browser defaults * add basic readme with deltas
* add button as a new web component * change files * add package export * add icon attr, fixup disabled styles * update color => fill * add disabled focusable click handling * move to attribute syntax for aria-disabled * remove icon attribute in favor of min-height to accommodate icons * use long form for padding to ensure we catch browser defaults * add basic readme with deltas
* add button as a new web component * change files * add package export * add icon attr, fixup disabled styles * update color => fill * add disabled focusable click handling * move to attribute syntax for aria-disabled * remove icon attribute in favor of min-height to accommodate icons * use long form for padding to ensure we catch browser defaults * add basic readme with deltas
* add button as a new web component * change files * add package export * add icon attr, fixup disabled styles * update color => fill * add disabled focusable click handling * move to attribute syntax for aria-disabled * remove icon attribute in favor of min-height to accommodate icons * use long form for padding to ensure we catch browser defaults * add basic readme with deltas
* add button as a new web component * change files * add package export * add icon attr, fixup disabled styles * update color => fill * add disabled focusable click handling * move to attribute syntax for aria-disabled * remove icon attribute in favor of min-height to accommodate icons * use long form for padding to ensure we catch browser defaults * add basic readme with deltas
* add button as a new web component * change files * add package export * add icon attr, fixup disabled styles * update color => fill * add disabled focusable click handling * move to attribute syntax for aria-disabled * remove icon attribute in favor of min-height to accommodate icons * use long form for padding to ensure we catch browser defaults * add basic readme with deltas
* add button as a new web component * change files * add package export * add icon attr, fixup disabled styles * update color => fill * add disabled focusable click handling * move to attribute syntax for aria-disabled * remove icon attribute in favor of min-height to accommodate icons * use long form for padding to ensure we catch browser defaults * add basic readme with deltas
* add button as a new web component * change files * add package export * add icon attr, fixup disabled styles * update color => fill * add disabled focusable click handling * move to attribute syntax for aria-disabled * remove icon attribute in favor of min-height to accommodate icons * use long form for padding to ensure we catch browser defaults * add basic readme with deltas
* add button as a new web component * change files * add package export * add icon attr, fixup disabled styles * update color => fill * add disabled focusable click handling * move to attribute syntax for aria-disabled * remove icon attribute in favor of min-height to accommodate icons * use long form for padding to ensure we catch browser defaults * add basic readme with deltas
* add button as a new web component * change files * add package export * add icon attr, fixup disabled styles * update color => fill * add disabled focusable click handling * move to attribute syntax for aria-disabled * remove icon attribute in favor of min-height to accommodate icons * use long form for padding to ensure we catch browser defaults * add basic readme with deltas
* add button as a new web component * change files * add package export * add icon attr, fixup disabled styles * update color => fill * add disabled focusable click handling * move to attribute syntax for aria-disabled * remove icon attribute in favor of min-height to accommodate icons * use long form for padding to ensure we catch browser defaults * add basic readme with deltas
* add button as a new web component * change files * add package export * add icon attr, fixup disabled styles * update color => fill * add disabled focusable click handling * move to attribute syntax for aria-disabled * remove icon attribute in favor of min-height to accommodate icons * use long form for padding to ensure we catch browser defaults * add basic readme with deltas
* add button as a new web component * change files * add package export * add icon attr, fixup disabled styles * update color => fill * add disabled focusable click handling * move to attribute syntax for aria-disabled * remove icon attribute in favor of min-height to accommodate icons * use long form for padding to ensure we catch browser defaults * add basic readme with deltas
Previous Behavior
New Behavior
Related Issue(s)