StackExchange / Stacks

Stack Overflowโ€™s Design System
https://stackoverflow.design
MIT License
602 stars 90 forks source link

feat(topbar): add popover child component #1739

Closed dancormier closed 1 month ago

dancormier commented 1 month ago

STACKS-613

This PR adds a Stacks topbar popover component so we can control the size and overflow behavior, primarily to meet the success criteria for WCAG SC 1.4.0 Reflow.

[!NOTE] I still need to add tests to this PR, but I'm opening it for review to get eyes on it earlier rather than later.

New classes

This PR introduces the following child component classes to the topbar:

Concessions

Since the topbar height is set with --theme-topbar-height, we know the height of the topbar for certain, but we cannot know for sure what the height of the search group is when it's rendered below the topbar (in the case of small viewports). Because of this, I took an educated guess and yielded on the side of caution to ensure the popover is rendered within the viewport.

It would be unlikely consumers would manually alter the height of the search group in mobile in such a way that it's significantly larger than the default, potentially rendering a portion of the popover outside of the viewport boundaries. I think this is a safe approach but I'm willing to reconsider if objections arise.

TODO

Screenshots

320 x 256 ![20240523-052206](https://github.com/StackExchange/Stacks/assets/647177/bf6fc47d-7c0d-4509-8dd6-a74efbc4e1fc)
770 x 256 ![20240523-052050](https://github.com/StackExchange/Stacks/assets/647177/edf8e78d-d07f-4724-8214-39f6da0477cf)

To test

For ease of testing this PR, I've added the popover to the main topbar of the Stacks docs site. It's temporary and will be removed before merging this PR.

Bonus if you're ambitious (I don't necessarily recommend it, though)

netlify[bot] commented 1 month ago

Deploy Preview for stacks ready!

Name Link
Latest commit 794a841b80a3548fa5a206d9ef1e2a786c466449
Latest deploy log https://app.netlify.com/sites/stacks/deploys/6658cd27f211ff0008598e90
Deploy Preview https://deploy-preview-1739--stacks.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

dancormier commented 1 month ago

@CGuindon

For the overlapping issue that should be Stacks only, would this same issue appear on Teams? They also have both the helper and results โ€” would those two popovers now overlap?

On Teams, there's just a single popover that gets its contents swapped out when results render. It shouldn't be an issue there.

Also, I don't know how to test this with the overlapping of two popovers? I can't see the results on small screens.

I should've specified that the Algolia search suggestions popover is not a native Stacks popover so exclude that from your testing. I added a dummy popover to the search for demonstration purposes and plan to remove it once this PR is approved, and that's the one I recommend testing against (you can get only that popover to show by only focusing the search input)