Sets aria-disabled on individual tab when parent Tabs is disabled #31239
+45
−0
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
chrisdholt
reviewed
Apr 30, 2024
chrisdholt
reviewed
Apr 30, 2024
| * 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.
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 |
fabricteam
reviewed
Apr 30, 2024
| @@ -0,0 +1,7 @@ | |||
| { | |||
There was a problem hiding this comment.
🕵 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. |
chrisdholt
requested changes
Apr 30, 2024
| 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.
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.
microsoft-github-policy-service
bot
added
Needs: Author Feedback
and removed
Needs: Author Feedback
labels
radium-v
force-pushed
the
web-components-v3
branch
from
3287714
to
3d89250
Compare
procload
requested review from
a team,
GeoffCoxMSFT,
marcosmoura,
sopranopillow,
tomi-msft,
micahgodbolt,
smhigley,
behowell,
spmonahan,
khmakoto,
dzearing,
Jacqueline-ms,
robtaft-ms and
Hirse
as code owners
…is set to disabled
procload
force-pushed
the
users/procload/disabledTabFix
branch
from
6fc8240
to
8e375fc
Compare
Hotell
removed request for
a team,
dzearing,
marcosmoura,
micahgodbolt,
Hirse,
robtaft-ms,
smhigley,
sopranopillow,
khmakoto,
jurokapsiar,
GeoffCoxMSFT,
Jacqueline-ms,
JustSlone,
behowell,
tomi-msft,
spmonahan and
tudorpopams
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.