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
Sets aria-disabled on individual tab when parent Tabs is disabled #31239
base: master
Are you sure you want to change the base?
Conversation
* Iterates over each tab element and sets its `disabled` attribute based on the `disabled` property of the `Tabs` component. | ||
* If the `disabled` property is `true`, the `disabled` attribute of the tab element will be set to `'true'`; otherwise, it will be set to `'false'`. | ||
*/ | ||
protected setTabs(): void { |
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.
Should this be done on disabledChanged
given that's an attribute? I wouldn't enter the for loop unless disabled is true or disabled is changing to false.
📊 Bundle size report✅ No changes found |
@@ -0,0 +1,7 @@ | |||
{ |
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.
🕵 fluentui-web-components-v3 Open the Visual Regressions report to inspect the affected screenshots
Accordion 3 screenshots
Image Name | Diff(in Pixels) | Image Type |
---|---|---|
Accordion.Size - RTL.normal.chromium.png | 2 | Changed |
Accordion.Size - Dark Mode.normal.chromium.png | 2 | Changed |
Accordion.Size.normal.chromium.png | 2 | Changed |
Avatar 5 screenshots
Image Name | Diff(in Pixels) | Image Type |
---|---|---|
Avatar.Active.normal.chromium.png | 5 | Changed |
Avatar.Active Appearance.normal.chromium.png | 10 | Changed |
Avatar.Active Appearance - Dark Mode.normal.chromium.png | 2 | Changed |
Avatar.Color - RTL.normal.chromium.png | 1 | Changed |
Avatar.Color.normal.chromium.png | 1 | Changed |
Badge 6 screenshots
Image Name | Diff(in Pixels) | Image Type |
---|---|---|
Badge.Default.normal.chromium.png | 1 | Changed |
Badge.Appearance - Dark Mode.normal.chromium.png | 15 | Changed |
Badge.Shape.normal.chromium.png | 1 | Changed |
Badge.Appearance.normal.chromium.png | 1 | Changed |
Badge.Size.normal.chromium.png | 6 | Changed |
Badge.Color.normal.chromium.png | 1 | Changed |
Button 15 screenshots
Image Name | Diff(in Pixels) | Image Type |
---|---|---|
Button.Large Icon Only - Dark Mode.hover.chromium.png | 3 | Changed |
Button.Large Icon Only.hover.chromium.png | 4 | Changed |
Button.Large Icon Only.pressed.chromium.png | 3 | Changed |
Button.Large Icon Only.default.chromium.png | 5 | Changed |
Button.Large Icon Only - Dark Mode.pressed.chromium.png | 5 | Changed |
Button.Large With Icon - Dark Mode.default.chromium.png | 4 | Changed |
Button.Large Icon Only - Dark Mode.default.chromium.png | 4 | Changed |
Button.Large With Icon - Dark Mode.pressed.chromium.png | 5 | Changed |
Button.Large With Icon - RTL.default.chromium.png | 5 | Changed |
Button.Large With Icon.pressed.chromium.png | 3 | Changed |
Button.Large With Icon.hover.chromium.png | 4 | Changed |
Button.Large With Icon - RTL.hover.chromium.png | 4 | Changed |
Button.Large With Icon - RTL.pressed.chromium.png | 3 | Changed |
Button.Large With Icon - Dark Mode.hover.chromium.png | 3 | Changed |
Button.Large With Icon.default.chromium.png | 5 | Changed |
Divider 3 screenshots
Image Name | Diff(in Pixels) | Image Type |
---|---|---|
Divider.With Svg - RTL.normal.chromium.png | 3 | Changed |
Divider.With Svg.normal.chromium.png | 3 | Changed |
Divider.Vertical With Svg.normal.chromium.png | 4 | Changed |
MenuList 16 screenshots
Image Name | Diff(in Pixels) | Image Type |
---|---|---|
MenuList.Checkbox With Icons - RTL.1st selected.chromium.png | 4 | Changed |
MenuList.Checkbox With Icons.1st selected.chromium.png | 4 | Changed |
MenuList.Checkbox With Icons - RTL.2nd selected.chromium.png | 5 | Changed |
MenuList.Radio With Icons - RTL.1st selected.chromium.png | 4 | Changed |
MenuList.Checkbox With Icons - RTL.normal.chromium.png | 4 | Changed |
MenuList.Radio With Icons - RTL.normal.chromium.png | 4 | Changed |
MenuList.Checkbox.2nd selected.chromium.png | 1 | Changed |
MenuList.Radio With Icons - RTL.2nd selected.chromium.png | 4 | Changed |
MenuList.With Icons.hover menuitem.chromium.png | 3 | Changed |
MenuList.Checkbox With Icons.normal.chromium.png | 4 | Changed |
MenuList.Radio With Icons.2nd selected.chromium.png | 4 | Changed |
MenuList.Radio With Icons.normal.chromium.png | 4 | Changed |
MenuList.Radio With Icons.1st selected.chromium.png | 4 | Changed |
MenuList.Checkbox With Icons.2nd selected.chromium.png | 5 | Changed |
MenuList.Checkbox - RTL.2nd selected.chromium.png | 1 | Changed |
MenuList.With Icons.normal.chromium.png | 4 | Changed |
Slider 2 screenshots
Image Name | Diff(in Pixels) | Image Type |
---|---|---|
Slider.Size Small.normal.chromium.png | 1 | Changed |
Slider.Size Small.rightArrow.chromium.png | 1 | Changed |
Switch 2 screenshots
Image Name | Diff(in Pixels) | Image Type |
---|---|---|
Switch.Checked.hover.chromium.png | 1 | Changed |
Switch.Checked - RTL.hover.chromium.png | 1 | Changed |
Text 1 screenshots
Image Name | Diff(in Pixels) | Image Type |
---|---|---|
Text.Block.normal.chromium.png | 995 | Changed |
TextInput 1 screenshots
Image Name | Diff(in Pixels) | Image Type |
---|---|---|
TextInput.Size Large.normal.chromium.png | 2 | Changed |
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. |
this.tabs.forEach((tab: HTMLElement) => { | ||
// Only set the 'disabled' attribute if it's not already set on the individual tab | ||
if (!tab.hasAttribute('aria-disabled')) { | ||
tab.setAttribute('aria-disabled', this.disabled ? 'true' : 'false'); |
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.
In this case you can just set the disabled attribute instead. That maps in the template as a boolean attr and will set aria-disabled.
3287714
to
3d89250
Compare
…is set to disabled
6fc8240
to
8e375fc
Compare
Previous Behavior
Setting the disabled state of the Tabs component did not set the aria-disabled state of the individual tab to true.
New Behavior
This sets the aria-disabled attribute to true when its parent Tabs component is set to disabled.