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

Query filters: Fix scrollbar overlaying dropdown menu #473

Merged
merged 2 commits into from
Oct 16, 2023

Conversation

renintw
Copy link
Contributor

@renintw renintw commented Oct 12, 2023

Fixes #467 (To be specific, #467 (comment))

Related: WordPress/wporg-showcase-2022#248

This PR adds a toggle function for overflow-x. When a theme assigns overflow-x: scroll to the query filter, change it to overflow-x: hidden when the dropdown menu is opened. And if it's closed, change it back to scroll.

Approach here was employed due to the overlapping issue can't be easily resolved just by adjusting the z-index or position of the query filter and the dropdown menu for a few potential reasons:

  1. Query Filter is the parent of Dropdown Menu.
  2. Dropdown menu uses position: fixed.
  3. Behavior of the Scrollbar is determined by browser or OS.

Screenshots

Before After
before after

@renintw
Copy link
Contributor Author

renintw commented Oct 12, 2023

Context:
When on a mobile screen, due to the horizontal overflow, a persistent scrollbar has been set up via CSS to inform users there's more content to the right. This configuration led to the issue mentioned in this PR: the scrollbar overlays the dropdown menu.

Current Situation:
This PR addresses the overlapping scrollbar issue. However,

  1. Steve mentioned here that always displaying the scrollbar looks a bit odd visually. I think the length of the scrollbar might also contribute to this awkward appearance.
image
  1. On my Mac using Chrome and resizing the window, I can always see the scrollbar. But when I switch to my phone (iPhone 12), the scrollbar disappears completely. As a result, on the Archive page, one would be completely unaware of the existence of the Sort filter. This can be tested on showcase-v2.

I guess the simplest solution here is to let the filters wrap, as shown in the video below. This method also addresses the issues raised in this PR.
A more complex solution might be rethinking the arrangement of the filters or thinking about ways to hint to users that there's more content to the right.

Thoughts on this? @jasmussen @marko-srb

wrap.mp4

Copy link
Contributor

@adamwoodnz adamwoodnz left a comment

Choose a reason for hiding this comment

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

👍 Works for me, with a small change 🙂

@adamwoodnz
Copy link
Contributor

Thoughts on this? @jasmussen @marko-srb

Or @WordPress/meta-design

@jasmussen
Copy link
Collaborator

I think @fcoveram's intent with the filter bar design he worked on was for them to scroll horizontally, and if that is the case, this z-index fix seems good. But I'd love if he had time to take a look. Depending on that feedback, letting the dropdowns stack seems acceptable to me as well.

@StevenDufresne
Copy link
Contributor

I think the design works fine, but we could revisit the type of affordance.

Youtube comes to mind as an example:
Screenshot 2023-10-16 at 12 05 27 PM

@fcoveram
Copy link
Collaborator

@jasmussen is right about the horizontal intention.

To give more context, the idea of showing search and filter actions in two bars began with the need for displaying the number of results by a11y reasons and placing apart the sort action, as the latter is not a filter. In most cases, especially in the index family's landing page (Showcase, Themes, Patterns, and Plugins), the page is a curated list of items, whereas the search result/archive allows users to play with filters and sorting. Therefore, outlining these two use cases reinforced the context difference.

Since the layout changed in wporg-showcase-2022#167, I'm drawn to follow @StevenDufresne's reference. I thought about it during the design, yet the sort action will be placed at the end of the list in a filter-actions context.

If we consider the two bars idea valid, I'm happy to create a ticket to address it.

@renintw renintw force-pushed the fix/on-mobile-the-scrollbar-appears-on-top-of-modal branch from 262f081 to b5bb9ef Compare October 16, 2023 18:33
@renintw
Copy link
Contributor Author

renintw commented Oct 16, 2023

@fcoveram thanks for the details. The two bars idea makes sense to me, I reckon we can follow Steve's example and this is supposed to be shipped before launching?

If not, the simpler solution (let the filters wrap) should be used first. This will only bring benefits (users with smaller screens can see the Sort option), with no downsides (the experience for users with larger screens remains unchanged).

@renintw renintw requested a review from adamwoodnz October 16, 2023 18:59
Copy link
Contributor

@adamwoodnz adamwoodnz left a comment

Choose a reason for hiding this comment

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

LGTM in it's current state

@renintw
Copy link
Contributor Author

renintw commented Oct 16, 2023

I'm going to merge this PR first as the original issue has been resolved.
@fcoveram feel free to continue leaving your feedback here or open a new ticket for this if you prefer. 🙏

@renintw renintw merged commit 2f37a46 into trunk Oct 16, 2023
@renintw renintw deleted the fix/on-mobile-the-scrollbar-appears-on-top-of-modal branch October 16, 2023 23:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Query Filter [Type] Bug Something isn't working
Development

Successfully merging this pull request may close these issues.

Query Filter: On mobile, horizontal scroll bar is visible on page load.
5 participants