Skip to content

Change close button on modal to absolute positioned have correct tab order#1715

Merged
just-be-dev merged 2 commits intomainfrom
update-modal-focus
Aug 8, 2023
Merged

Change close button on modal to absolute positioned have correct tab order#1715
just-be-dev merged 2 commits intomainfrom
update-modal-focus

Conversation

@just-be-dev
Copy link
Copy Markdown
Contributor

@just-be-dev just-be-dev commented Aug 8, 2023

Fixes #1714

@vercel
Copy link
Copy Markdown

vercel bot commented Aug 8, 2023

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

Name Status Preview Updated (UTC)
console ✅ Ready (Inspect) Visit Preview Aug 8, 2023 6:36pm

@benjaminleonard
Copy link
Copy Markdown
Contributor

Just tweaked it — there's some extra spacing because of the hover state and the increased click area. This now matches the previous version.

LGTM

Thanks!

</Dialog.Title>
)}
{children}
<Dialog.Close
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We might want to add a comment for context like the side modal — in case this component is recreated and we hit the same issue again.

@just-be-dev just-be-dev merged commit f84dc87 into main Aug 8, 2023
@just-be-dev just-be-dev deleted the update-modal-focus branch August 8, 2023 18:45
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.

Focus a default button instead of a close button in dialogs

2 participants