-
Notifications
You must be signed in to change notification settings - Fork 264
Always allow collapsing the drawer #271
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
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.
The only problem is that when I am making
drawerOpen trueI am able to show the drawer in desktop mode but I am not able to remove it when I am clicking on any sidebar list.
I see two issues:
- There seems to be 3
Drawercomponent being used inDashboard/index.jsx. We only need to have 1. You can remove the ones that have theHiddencomponent right above it. - The
Drawercomponent is missing theonCloseand the propervariantprop. That's why it's not being collapsing when clicking on a sidebar element. For a list of theDrawerprops, you can look at the material docs. Hint: thevariantshould not bepermanent.
|
@helfi92 thanks, I think it is ready for review. can you review this? :) |
helfi92
left a comment
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.
Good progress :) It turns out we need to handle the documentation site (anything under /docs) a bit differently. The drawer is pretty important for docs so we should keep it there. I added some changes in this commit. Please apply them and try to see what I did there. Feel free to copy paste the following two files: src/components/Dashboard/index.jsx src/components/NotFound/index.jsx.
|
@helfi92 I modify my code as you said and saw that previously for Documentation also we have a temporary variant drawer. But with these changes, we have a permanent drawer as it is important for a newcomer. But there is a problem in mobile view which I didn't like i.e( when we are clicking on the DocsSidebarList, drawer is still open and we have to click on the backdrop to close it.) What do you think and also what should I do now after these changes? |
|
You can try to figure it out. I'm also ok merging after you add the requested changes then we could fix the issue you pointed out in a different PR. |
|
@helfi92 I pushed the commit which is required. you can merge this now :) thanks for the help. |
helfi92
left a comment
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.
😍
Regarding issue #225
@helfi92 can you make some comment on this. The only problem is that when I am making
drawerOpen trueI am able to show the drawer in desktop mode but I am not able to remove it when I am clicking on any sidebar list. can you take a look or suggest me anything after looking this?