E532 fix(label): added support for disabled labels by mcoker · Pull Request #6424 · patternfly/patternfly · GitHub
[go: up one dir, main page]

Skip to content

fix(label): added support for disabled labels#6424

Merged
srambach merged 4 commits intopatternfly:mainfrom
mcoker:issue-6055
Mar 25, 2024
Merged

fix(label): added support for disabled labels#6424
srambach merged 4 commits intopatternfly:mainfrom
mcoker:issue-6055

Conversation

@mcoker
Copy link
Contributor
@mcoker mcoker commented Mar 16, 2024

fixes #6055

  • Moves the template hbs file to the root label component dir since the workspace doesn't auto-compile-n-reload on files nested in subdirs.
  • Sets the close button color in a disabled label to the same color as the disabled text, since by default a disabled close button color is the same color as the disabled label background.
  • Just setup to work on regular (filled/outline) labels - not the overflow or add. It's easy enough to add support for those, or we could add that support if/when it comes up. wdyt @lboehling @andrew-ronaldson?

@patternfly-build
Copy link
Collaborator
patternfly-build commented Mar 16, 2024

@srambach
Copy link
Member

@mcoker looks like a11y tests are failing on
92/266 /components/label
violations: duplicate-id-aria

Copy link
Member
@srambach srambach left a comment

Choose a reason for hiding this comment

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

Other than the duplicate ids, looks great! 🏷️


{{> label
label--isRemovable=true
label--id=(concat label-template-variants--id '-link-removable')
Copy link
Member

Choose a reason for hiding this comment

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

Probably want to add -disabled to the id?


{{> label
label--isRemovable=true
label--id=(concat label-template-variants--id '-clickable-removable')
Copy link
Member

Choose a reason for hiding this comment

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

Probably want to add -disabled to the id?

@mcoker mcoker requested a review from srambach March 19, 2024 02:20
Copy link
Member
@srambach srambach left a comment

Choose a reason for hiding this comment

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

🚀

Comment on lines +58 to +64
{{> label
label--isRemovable=true
label--id=(concat label-template-variants--id '-link-disabled')
label--IsDisabled=true
label-text--value=(concat label-template-variants--title ' link removable (disabled)')
label-content--IsLink=true
label-icon--value="info-circle"}}
Copy link
Contributor

Choose a reason for hiding this comment

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

Similar to my comment in the v6 PR about disabled link labels.

href="#"
{{#if label--IsDisabled}}
tabindex="-1"
aria-hidden="true"
Copy link
Contributor

Choose a reason for hiding this comment

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

Might be better to use aria-disabled here instead. Just thinking if it would be confusing for there to be a "close" button for something that is hidden from AT.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🤦‍♂️ totally meant for that to be aria-disabled, nice catch.

@mcoker mcoker requested a review from thatblindgeye March 21, 2024 15:08
@srambach srambach merged commit 4531528 into patternfly:main Mar 25, 2024
@patternfly-build
Copy link
Collaborator

🎉 This PR is included in version 5.3.0-prerelease.5 🎉

The release is available on:

Your semantic-release bot 📦🚀

@patternfly-build
Copy link
Collaborator

🎉 This PR is included in version 5.3.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Label] - allow clickable labels to be disabled

4 participants

0