-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
[Form] Reorder form markup to prevent VisuallyHidden bug #5181
Conversation
size-limit report
|
{submitMarkup} | ||
{children} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change doesn't impact implicitSubmit
functionality:
Screen.Recording.2022-02-15.at.16.45.39.mov
It also won't affect the order AT users encounter the markup - the VisuallyHidden
button here is aria-hidden
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Love it. Thank you @emma-boardman
@alex-page how do I get the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@emma-boardman shipping this |
WHY are these changes introduced?
The
top: 0
CSS was removed fromVisuallyHidden
in Polaris v7.4.0.In long forms that utilise the deprecated
Sheet
, the updated positioning ofVisuallyHidden
in aForm
component scrolls the underlying Form out of view when theSheet
is opened.In
online-store-web
, this scrolling was causing thePopover
inside theAddMenuItem
sheet to close immediately after opening.Full context from support: https://shopify.slack.com/archives/C8RBQFF6U/p1644009960815369
📹 Media
Before
Screen.Recording.2022-02-15.at.17.21.57.mov
After
Screen.Recording.2022-02-15.at.17.06.04.mov
WHAT is this pull request doing?
submitMarkup
andchildren
🎩 checklist