collapsible feature added #623
Conversation
|
@Rupeshiya @AuraOfDivinity please review |
There was a problem hiding this comment.
@rajprakash00 Thanks for the PR! This looks good. Just a couple of concerns from my end but I think we can get them sorted out.
-
Could you remove the unnecessary code formatting related changes from the code? It's harder to identify the code changes. A small tip: disable the format on save option if you're using vs code.
-
The sidebar is not responsive and breaks at smaller screen sizes. As of the current implementation, the sidebar turns into a small strip when viewed on smaller screens. Can you implement the same functionality for your changes as well?
-
I personally think we could use some design improvements as well. Maybe @jaskiratsingh2000 @Rupeshiya and Siddharth could provide some feedback on this?
| <button | ||
| onClick={this.handleViewSidebar} | ||
| className="sidebar-toggle" | ||
| style={ | ||
| sideBarClass === "sidebar-open" | ||
| ? { marginLeft: "13vw" } | ||
| : { marginLeft: 0 } | ||
| } | ||
| > |
There was a problem hiding this comment.
Isn't it possible to integrate this button itself to the sidebar itself without repeating it in all the files?
There was a problem hiding this comment.
Ya, i tried implementing it with navigation.js file but for some reason(i am guessing related to fixed position property) the sidebar collapses but the main content doesn't resizes(failing the sole purpose of collapse feature).
There was a problem hiding this comment.
@AuraOfDivinity for your 2nd point I removed the collapsible feature for small sizes(since it was unnecessary in small screens).
Also i was thinking can we not create navigation as a uniform component in layout folder?
|
@rajprakash00 any updates on this PR? |
|
@AuraOfDivinity I think I've done some of your requested changes in the updated commit,is there any more changes required? |
Fixed #622
Added collapsible features for pages
Screenshot:
Demo: https://deploy-preview-623--friendly-sinoussi-3b191b.netlify.app