Improvements to Left Side Menu #125
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Unit tests are randomly breaking? It works on my machine: |
This commit updates the left side menu to be more consistent with the mobile app's design. These changes are mainly in the CSS and HTML, though there is one variable added to the component's state to manage the new expandable Apps menu. PER-8667: Improvements to left side menu
meisekimiu
force-pushed
the
left-menu-redesign
branch
from
de5dc56 to
71c72da
Compare
jasonaowen
reviewed
May 26, 2022
This commit stores the state of the different left menu submenus in SessionStorage so that they persist throughout page reloads and refreshes. Especially with the redesign, it feels odd that they close when moving between pages. This also includes a quick change to not show the dropdown toggle if there are no Apps subfolders. PER-8667: Improvements to left side menu
Within the template HTML file, new changes made the lines a bit too long. Within the Typescript file, there were a whole host of things my IDE were complaining about so I went and fixed all of those while I was cleaning up the HTML and implementing some of the other fixes. PER-8667: Improvements to left side menu
meisekimiu
force-pushed
the
left-menu-redesign
branch
from
0072fd4 to
ffef6fd
Compare
|
I've addressed the things you've brought up, @jasonaowen, including the line length concerns (though we really should be using Prettier because I did this manually and I'm not sure I did it the "right" way; I would love to not have to even think about this in the future). I also cleaned up some of the other things my IDE was yelling at in the TypeScript file. |
jasonaowen
approved these changes
May 27, 2022
There was a problem hiding this comment.
This is fantastic, thanks @meisekimiu! The formatting looks good, although yeah not sure if Prettier will disagree - +1 to setting all that up ASAP.
I was able to run this locally in both desktop and mobile views, and it looks and feels great!
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR updates the left side menu to be more consistent with the mobile app's design. One thing that wasn't really specified in the zeplin designs was what this should look like in mobile view. The layout of the mobile view is kept intact, just including the new icon and color changes.
Resolves PER-8667.