Page MenuHomePhabricator

[Cashtab] Add hamburger menu to taskbar
ClosedPublic

Authored by kieran709 on Sep 15 2022, 19:48.

Details

Reviewers
bytesofman
emack
Group Reviewers
Restricted Project
Commits
rABCeb90042170f6: [Cashtab] Add hamburger menu to taskbar
Summary

Related to T2376. Added menu icon that animates from hamburger to X on click, replaced the cog in the navbar with the hamburger menu. Clicking hamburger menu icon expands the nav menu.

Test Plan

cd web/cashtab && npm start
observe the hamburger icon in the navbar
click the hamburger icon
observe that the icon animates to an X
observe the NavMenu transition height
hover over the settings NavItem
observe that the color turns to blue on hover
click the settings NavItem
observe that the NavMenu closes on click
observe that clicking the NavItem will redirect to Settings page.

Diff Detail

Repository
rABC Bitcoin ABC
Branch
add-hamburger-menu-to-navbar
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 20053
Build 39794: Build Diffcashtab-tests
Build 39793: arc lint + arc unit

Event Timeline

emack requested changes to this revision.Sep 16 2022, 16:01

image.png (74×475 px, 3 KB)

The burger seems slightly wider and longer than the other nav icons, height and width just needs to be reduced a fraction to line up with the first 3 icons.

image.png (102×497 px, 6 KB)

while in expanded mode:

  • the X's height and width also needs to be reduced a fraction
  • when I tested this in mobile mode, the Settings option was rather small to click on both iOS and Android devices. This will be especially evident when there are multiple navItems.

image.png (404×312 px, 157 KB)

Here you can see its scale on an iphone relative to a standard sized pen.
Try expanding its height and then testing on mobile. You can test this by going to d11997.netlify.app on your iOS or Android device.

image.png (191×499 px, 11 KB)

  • when I added a few additional navItems with different char length than "Settings" the result seems to be center justified - they should all align right, with the icons all in a straight vertical line.
web/cashtab/src/components/App.js
174

NavIcon would be a better name and for consistency

This revision now requires changes to proceed.Sep 16 2022, 16:01

Fixed height and width of hamburger & X icons, increased height of NavItems, NavItems align right with all icons in a vertical line, Icon name changed to NavIcon.

Nice one doing this without any npm packages!

  1. Let's put Airdrop behind the hamburger as well
  2. Needs a border or some kind of aesthetic/design consideration...blends in too much with the background
  3. Looks a bit small for picking out individual options with a thumb (i.e. the options that are revealed by the hamburger look small
  4. Make sure to also add this to the extension App.js
This revision now requires changes to proceed.Sep 21 2022, 23:16

responding to review feedback

Nice

image.png (303×545 px, 36 KB)

  1. Right border of this drawer should exactly line up with right border of the app. Right now looks like there are a few pixels in between.
  2. Can you keep the border around the whole drawer but not in between drawer items? i.e. remove the line between 'settings' and 'airdrop' here.
  3. Change the transition effect. Would like to see the drawer take 1s to open and close (with it sliding up and down over that interval)
This revision now requires changes to proceed.Sep 22 2022, 20:27

Per review feedback: widened NavMenu, added border to NavMenu & removed borders from NavItems. Also, added additional transition effects to handle the new border.

Cleaned up timing issue on close. When transitioning max-height, even though the element does not grow to that height, the transition still happens from that height. For this reason, the closing transitions have been shortened considerably and cubic-bezier is used to smooth the transition effect on close.

I'm still seeing the dividing line in the extension

image.png (322×396 px, 26 KB)

This revision now requires changes to proceed.Sep 24 2022, 23:17

Responding to review feedback.

The build failed due to an unexpected infrastructure outage. The administrators have been notified to investigate. Sorry for the inconvenience.
bytesofman added inline comments.
web/cashtab/src/components/App.js
210 ↗(On Diff #35186)

This is 8.5rem above in the extension version -- why the discrepancy?

This revision now requires changes to proceed.Sep 26 2022, 20:56

responding to review feedback

This revision is now accepted and ready to land.Sep 27 2022, 21:43
This revision was automatically updated to reflect the committed changes.