Skip to content

Conversation

@frankieyan
Copy link
Member

@frankieyan frankieyan commented May 30, 2022

Short description

Ariakit's menu renders the menu list with a menubar role instead of menu in Safari due to a VoiceOver issue. As we leverage the role attribute for styling, this resulted in them not being applied.

Before After
image image

PR Checklist

  • Added tests for bugs / new features
  • Updated docs (storybooks, readme)
  • Executed npm run validate and made sure no errors / warnings were shown
  • Described changes in CHANGELOG.md
  • Bumped version in package.json and package-lock.json (npm --no-git-tag-version version <major|minor|patch>) ref
  • Updated all static build artifacts (npm run build-all)

Versioning

Patch

@frankieyan frankieyan requested review from a team and Bloomca and removed request for a team May 30, 2022 15:28
Copy link
Contributor

@Bloomca Bloomca left a comment

Choose a reason for hiding this comment

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

Looks good.

I checked and it seems we don't use menubar in Todoist/Twist, so should work fine.

I checked other components in Safari, and all looks good as well. I found one small issue in Tooltip's story -- tab from the input will not focus the button, but hovering works.

@frankieyan
Copy link
Member Author

@Bloomca do you mean this one?
image

It seems to work for me 👀 but I'm on Big Sur still, using Safari Technology Preview (Release 139 (Safari 15.4, WebKit 16613.1.14.41.2))

@Bloomca
Copy link
Contributor

Bloomca commented May 31, 2022

@frankieyan yes, that's the one. My Safari's version is Version 15.4 (17613.1.17.1.13)

I mostly mentioned so it is documented somewhere, not blocking 🙌

@frankieyan
Copy link
Member Author

I updated both versions and can still tab to the button and show the tooltip 👀 the versions are:

  • TP Release 146 (Safari 15.4, WebKit 16614.1.14.10.6)
  • Version 15.5 (16613.2.7.1.9, 16613)

I'll maybe tag you once I pull this into TD to see if you still see the issue there 🙏

@frankieyan frankieyan merged commit c46c095 into main Jun 2, 2022
@frankieyan frankieyan deleted the frankie/fix-menu-styling branch June 2, 2022 11:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants