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(structure): intent menu item nodes not rendering #5728

Merged
merged 4 commits into from
Feb 20, 2024

Conversation

rexxars
Copy link
Member

@rexxars rexxars commented Feb 14, 2024

Description

Fixes #4891

Menu items in structure nodes that defined an intent instead of an action handler did not work after the facelift launch. There appears to be multiple different issues that caused this - I have addressed them in separate commits:

  1. The intent definition was not actually passed onto the structure nodes, leading to them being treated as a menu button without an action handler instead of an intent button
  2. The intent buttons did not actually render, because the as="a" rendered the entire button as a single link without any styling. I've modified it to use forwardedAs, but I am not confident that this approach is correct, or needs adjustment.

While investigating, I also found that the menu buttons got an aria label of This is disabled, even when not disabled. For now, I have disabled the aria label when not disabled, and will follow up with a PR to localize this string as well.

What to review

  1. Intent buttons work as expected. There is a test case in the test studio under the Untitled repro item: click the cogwheel in the next pane, which should open GRRM in a document editor if this works as intended.

Notes for release

  • Fixes an issue where menu items defined in structure that used intents would not work

@rexxars rexxars requested a review from a team as a code owner February 14, 2024 02:40
@rexxars rexxars requested review from bjoerge and removed request for a team February 14, 2024 02:40
Copy link

vercel bot commented Feb 14, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
performance-studio ✅ Ready (Inspect) Visit Preview Feb 14, 2024 7:48pm
test-studio ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 14, 2024 7:48pm
1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
studio-workshop ⬜️ Ignored (Inspect) Visit Preview Feb 14, 2024 7:48pm

Copy link
Contributor

No changes to documentation

Copy link
Contributor

github-actions bot commented Feb 14, 2024

Component Testing Report Updated Feb 14, 2024 7:48 PM (UTC)

File Status Duration Passed Skipped Failed
comments/CommentInput.spec.tsx ✅ Passed (Inspect) 31s 15 0 0
formBuilder/ArrayInput.spec.tsx ✅ Passed (Inspect) 5s 3 0 0
formBuilder/inputs/PortableText/Annotations.spec.tsx ✅ Passed (Inspect) 12s 3 0 0
formBuilder/inputs/PortableText/copyPaste/CopyPaste.spec.tsx ✅ Passed (Inspect) 12s 4 2 0
formBuilder/inputs/PortableText/Decorators.spec.tsx ✅ Passed (Inspect) 13s 6 0 0
formBuilder/inputs/PortableText/FocusTracking.spec.tsx ✅ Passed (Inspect) 32s 15 0 0
formBuilder/inputs/PortableText/Input.spec.tsx ✅ Passed (Inspect) 18s 9 0 0
formBuilder/inputs/PortableText/ObjectBlock.spec.tsx ✅ Passed (Inspect) 59s 18 0 0
formBuilder/inputs/PortableText/Styles.spec.tsx ✅ Passed (Inspect) 13s 6 0 0
formBuilder/inputs/PortableText/Toolbar.spec.tsx ✅ Passed (Inspect) 19s 9 0 0

Copy link
Member

@bjoerge bjoerge 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! ❤️

@rexxars rexxars added this pull request to the merge queue Feb 20, 2024
Merged via the queue into next with commit 613e1dd Feb 20, 2024
40 checks passed
@rexxars rexxars deleted the fix/4891/edit-intent-structure branch February 20, 2024 15:35
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.

Bug: Edit intent gets removed from menu item buttons
2 participants