-
Notifications
You must be signed in to change notification settings - Fork 3.3k
Filter by close icon added for mobile screen #12008
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
Conversation
@parlough I need help. When i don't use div for resource-filter-group-wrapper and use css given below then close functionality not works but filter_list icon in mobile screen show content every time when open/close slider .close-icon { Screen.Recording.2025-05-13.at.4.09.17.PM.mov |
Please review, @parlough. |
Thanks for working on this improvement @zalabhavy! Sorry that I haven't had a chance to review your PR yet, I've been away for the past week. I'll try to make time to review this tomorrow. I haven't had a chance to review your CSS or investigate yet, but from a quick look at the video, I'd consider implementing the behavior of the button by adding a click handler to the button with JS, similar to the existing handling for pressing the website/src/content/assets/js/learning-resources-index.js Lines 283 to 287 in a7876e7
|
Greetings from stale PR triage! Is this change still active? |
@Piinks Hello ! |
@parlough, I know you've been traveling, but any chance you can take a look at this? If not, should I close it? |
Sorry for the delay, I'll make sure to respond with help finishing this today! |
Sorry again about the delay @zalabhavy. I appreciate you raising this improvement and working on this! Are you still interesting in working on this or would you like me to push the final adjustments? If you are interested, similar to my previous comment, I suggest adding a normal button and manually adding a click handler to that with JS. So in the HTML, I'd add a button similar to the following in the header of the sidebar: <button id="close-filter-button" class="icon_button" aria-label="Close filter panel">
<span class="material-symbols icon-button" aria-hidden="true" translate="no">close</span>
</button> Then in the JS, in the const closeFilterButton = filtersEl.querySelector('#close-filters-button');
if (closeFilterButton) {
closeFilterButton.addEventListener('click', (_) => {
_closeMenu();
});
} There might be some other small CSS changes to make to make it look better and some other JS adjustments, but let me know if you need help with those or anything else. Thanks again! |
Okay @parlough , I’ll give it a try first and then share an update with you. |
Thanks for the update! Closing in favor of #12373. |
A close icon/button should be added to the "Filter by" wrapper on mobile screens to allow users to easily close the sidebar.
Fixed #11980
he close button works properly, but despite trying multiple times, I couldn't fix the issue where, after closing the filter once, clicking the filter icon again on a mobile screen doesn't show the filter content inside the card.
Screen.Recording.2025-05-13.at.3.56.50.PM.mov
Presubmit checklist