-
Notifications
You must be signed in to change notification settings - Fork 8
When the drawer is in ClipMode "Never" and Variant Temporary you can'… #42
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
base: master
Are you sure you want to change the base?
Conversation
…t close it. My suggestion is to also listen to click outside of it and close the drawer.
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.
Hello @bc-hvtk, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
Summary of Changes
This pull request addresses an issue where a temporary drawer, when in 'Never' clip mode, couldn't be closed by clicking outside of it. The solution implements a click outside handler to toggle the drawer off. Additionally, a test case was added to verify the fix.
Highlights
- Click Outside Handler: A new
handleOutsideClick
function is added toNavigationObserver.js
to listen for clicks outside the drawer and close it when necessary. - Event Listener: The
NavigationObserver.js
now adds and removes a click event listener to the document to handle clicks outside the drawer. - Test Case: A new test case
MudDrawerTemporary_ClickOutside_Closed
is added toNavMenuTests.cs
to ensure the drawer closes when clicking outside of it.
Changelog
- src/wwwroot/NavigationObserver.js
- Added
handleOutsideClick
function to toggle the drawer when clicking outside of it. - Added and removed a click event listener to the document to handle clicks outside the drawer (lines 56-57).
- Added logic to check if the click is within the drawer or on a drawer toggle element (lines 119-120).
- Added
- tests/StaticInput.UnitTests.Viewer/Components/Tests/NavMenu/NavMenuDrawerToggleNeverTest.razor
- Added a new Razor component to test the drawer behavior with
ClipMode.Never
andDrawerVariant.Temporary
.
- Added a new Razor component to test the drawer behavior with
- tests/StaticInput.UnitTests/Components/NavMenuTests.cs
- Added a new test case
MudDrawerTemporary_ClickOutside_Closed
to verify that clicking outside the drawer closes it.
- Added a new test case
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command>
or @gemini-code-assist <command>
. Below is a summary of the supported commands.
Feature | Command | Description |
---|---|---|
Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/
folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Did you know?
JavaScript was originally named Mocha, then LiveScript, before finally being named JavaScript.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
PR Summary
|
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.
Code Review
The pull request introduces a new feature to close the MudDrawer when clicking outside of it, specifically when the drawer is in 'Never' clip mode and 'Temporary' variant. This enhancement improves the user experience by providing a more intuitive way to dismiss the drawer. The implementation includes JavaScript changes to handle the click event and a new unit test to verify the functionality.
Summary of Findings
- Event Listener Management: The event listener is added and removed on every initialization. It would be better to only add it once and manage the state based on the drawer's properties.
- CSS Class Selector: The CSS class selector '.mud-drawer.mud-drawer--open' might be too specific and could be affected by other CSS rules. Consider using a more robust selector or a dedicated class for this behavior.
- Test Coverage: The new unit test covers the basic scenario, but it could be extended to include edge cases and different drawer configurations.
Merge Readiness
The pull request introduces a valuable feature and includes a unit test. However, the event listener management and CSS class selector could be improved. I recommend addressing these points before merging. I am unable to directly approve the pull request, and users should have others review and approve this code before merging.
@bc-hvtk I haven't had the chance to fully review this but wouldn't this close any drawer type and clip mode combination? I don't see anything limiting it to a specific clip mode or drawer type. Wouldn't a persistent drawer be closed with an outside click based on this change? |
@Anu6is You are absolutely right it doesn't. Nevertheless it covers the most used case in my opinion with one drawer that pops over the toggle. At least for mobile (XS) breakpoint this is quite common. If this introduce an unwanted behaviour it would fail other tests, wouldn't it? Although open to suggestion for the selector if you had something in mind 👍 (this is kind of a redneck fix IMO) |
At a minimum |
Will update as soon as I can ^^ |
…t close it. My suggestion is to also listen to click outside of it and close the drawer.