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.

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

fix: [M3-7208] Prevent forwarding the title prop to the markup #9739

Merged
merged 3 commits into from
Oct 3, 2023

Conversation

abailly-akamai
Copy link
Contributor

Description 📝

You've been annoyed by this on our modals?

Screenshot 2023-09-29 at 10 57 00

Look no further, this fix is for you.

The title prop on our modals creates more trouble than it's worth. It adds html tooltip (a proper HTML title feature) that follows you around and often obfuscate content or distracts the user.

It does not add any particular accessibility feature (known to me) and does not seem to be standard at all:
https://www.w3.org/WAI/ARIA/apg/patterns/dialog-modal/examples/dialog/

We already have the necessary features described in the accessibility standards docs so there's no need to continue defining them and require them on those components.

Major Changes 🔄

List highlighting major changes

  • Prevent forwarding the title prop to the html

How to test 🧪

  1. Open any modal/dialog in CM - example Linode resize
  2. Confirm the title tooltip is gone (usually after mouse pointing in the same spot for a couple seconds
  3. Confirm the title is removed from the markup
  4. Confirm the title is still being used wherever it should be (aria tags, modal title etc)

@abailly-akamai abailly-akamai marked this pull request as ready for review September 29, 2023 20:40
@abailly-akamai abailly-akamai requested a review from a team as a code owner September 29, 2023 20:40
@abailly-akamai abailly-akamai requested review from cpathipa and cliu-akamai and removed request for a team September 29, 2023 20:40
Copy link
Contributor

@mjac0bs mjac0bs left a comment

Choose a reason for hiding this comment

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

🎉 Thank you for making this change!

I was able to take a screenshot of a dialog for the first time without the tooltip popping up in the middle of it.
Screenshot 2023-10-02 at 10 46 08 AM

Also confirmed that the title was still rendering in dialogs as expected and we still have an ariaLabeledBy prop with the title of the dialog.

@mjac0bs mjac0bs added the Add'tl Approval Needed Waiting on another approval! label Oct 2, 2023
@bnussman-akamai bnussman-akamai added Approved Multiple approvals and ready to merge! and removed Add'tl Approval Needed Waiting on another approval! labels Oct 3, 2023
Copy link
Contributor

@cpathipa cpathipa left a comment

Choose a reason for hiding this comment

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

Nice fix! Confirming title tooltip are gone in modals and Modal title shows tooltip.
After:
image
Before:
image

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Approved Multiple approvals and ready to merge!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants