-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
New layout for mobile menu is ready. #2665
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.
@YegorSan as with previous PR too many unnecessary changes. Please check https://github.com/poanetwork/blockscout/pull/2665/files. Also, you can check which changes do you make before committing them.
Deleted untracked file from my code editor.
@vbaranov I have re-checked the files, should be better now. I am sorry, again VSCode turned on formatting for css files automatically. |
Added some changes to fix ESLINT issues
@vbaranov Sorry for such a huge list of commits. Lot of tests didn't pass yesterday (ESLINT, test_geth_http_websocket, etc). I've fixed all the issues. |
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.
-
I think elements in the header shouldn't jump on open menu https://www.loom.com/share/86f4fe08df8343b494054f478a390e83
-
Menu items are not centred as in @pashagonchar screenshot in the issue Menu mobile view #2653. I don't know should they be centred finally or not. @pashagonchar please check
@@ -30,7 +44,7 @@ | |||
</span> | |||
<%= gettext("Blocks") %> | |||
</a> | |||
<div class="dropdown-menu" aria-labelledby="navbarBlocksDropdown"> | |||
<div class="dropdown-menu" id="checkIfSmall" aria-labelledby="navbarBlocksDropdown"> |
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.
@YegorSan unnecessary trailing whitespace
@vbaranov Yes, I updated mockup in zeplin |
@vbaranov Hello. I am working on performing mentioned above changes right now. Will try to resolve in the shortest terms. |
@vbaranov Hi Victor. Just finished with the requested updates. All changes were performed. Please review. Thank you in advance. |
margin-right: -7px !important; | ||
padding-right: 0px; | ||
height: 20px!important; | ||
border-left: 1px solid #828ba0a1!important; |
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.
@YegorSan is the correct color hex here? It looks like it is invalid colour hex. It should be 3-byte hexadecimal code
@YegorSan in addition, elements are still jumping https://www.loom.com/share/71119ba076624298aeace00fc02d2a5e |
@YegorSan in addition, I don't see dark mode switcher in the desktop mode anymore |
@vbaranov Hello. I've added fixed height for logo (was max-height: 28px now height: 28px). Jumping issue is resolved now. |
@YegorSan please resolve merging conflicts |
@vbaranov Merging conflicts seems to be solved. Could you please review? Thank's in advance. |
@YegorSan changelog entry... |
@vbaranov I am sorry. Changelog is updated. |
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.
Dark mode switcher is not clickable in a mobile view
@vbaranov Hello Victor. I am sorry again, forgot to add separate JS onClick function for new button created. Now button is clickable and working as it expected. |
This PR is related to #2653 issue.
Motivation
New layout for mobile menu was requested.
Changelog
Created a new menu layout for mobile view.
*Not active state:
*
Active state:
Checklist for your PR
CHANGELOG.md
with this PRmaster
in theVersion
column.