[VisuallyHidden] Reintroduce VisuallyHidden top position

[emma-boardman]

WHY are these changes introduced?

top:0 was removed from VisuallyHidden in Polaris v7.4.0. Unfortunately, this change does not play well with Sheets using Popovers.

Prior to the top:0 change, when a Sheet was opened, the underlying content would not scroll. After the top:0 was removed, the VisuallyHidden elements, presumably by nature of them being position: absolute and outside the underlying page DOM flow, stretch the height of the underlying page when data-scroll-lock sets the body height to 100%. This causes the underlying page to scroll, which introduces strange Popover behaviour.

In https://github.com/Shopify/online-store-web, there are 2 impacted Sheet components. The first, we were able to solve by reordering the markup. The 2nd is harder - there are a lot of VisuallyHidden elements on the underlying page, and overriding the CSS of each element would be a lot of hacky code.

Sheet is deprecated, but until we have UX/dev resources to move away from the pattern, it would be really helpful to reintroduce top:0.

Before:

Screen.Recording.2022-02-17.at.17.43.43.mov

After:

(running fix on v8 Polaris)

Screen.Recording.2022-02-23.at.11.27.06.mov

Deployment 🌶️

If possible, it would amazing to cut a v8 hotfix with this change. Please let me know your thoughts 🙏

Other approaches considered

We could keep the a11y gains of removing top:0, and see if there's an adjustment to be made in Sheet. However, I don't know if it's worth sinking the time into a deprecated component. Again, would love to hear people's thoughts 🙏

How to 🎩

• In the Admin, navigate to Online Store > Themes
• Add themes to your theme library. You need enough themes that the Theme Library is extended below the fold
• Scroll a little from the top of the page.
• Add Theme > Connect to GitHub
• When the Sheet opens, the underlying page should not scroll.

🎩 checklist

• Tested on mobile
• Tested on [multiple browsers](MacOS: safari, firefox, chrome)

[github-actions]

size-limit report

Path	Size
cjs	156.29 KB (0%)
esm	88.35 KB (0%)
esnext	137.49 KB (+0.01% 🔺)
css	36.53 KB (+0.03% 🔺)

[utkarshsaxena93]

a11y gains of removing top:0

What are the a11y gains of removing top:0 ?

I'm good to ship this fix and creating a follow up issue to remove the deprecated Sheet component. Do you know which is the new component that we will use instead of the Sheet?

[sophschneider]

@utkarshsaxena93 #4641

[sophschneider]

Ideally there would be a way to prevent developers from creating markup using VisuallyHidden that would cause these kinds of issues without top: 0, not sure what the next steps for that would be.

[emma-boardman]

@alex-page a couple of question re: merge & release:

1. Is it possible to release this as a v8 hotfix? The bug captured in the PR description is in production, so we'd like to ship asap, if possible.

2. How do I bypass the Required Test (16.9.1) check? It blocks merging, but I don't think any CI runs on it since this change: https://github.com/Shopify/polaris-react/pull/4821/files#diff-b803fcb7f17ed9235f1e5cb1fcd2f5d3b2838429d4368ae4c57ce4436577f03fL11