frontend: move mobile settings into bottom navigation - #4331
Conversation
66eece7 to
72fb323
Compare
thisconnect
left a comment
thisconnect
left a comment
There was a problem hiding this comment.
tested Android & macOS LGTM with 1 nit.
| ); | ||
|
|
||
| export const MoreIconSVG = () => ( | ||
| export const SettingsIconSVG = () => ( |
There was a problem hiding this comment.
Nit: could you just use CogBlue from Icon componet?
There was a problem hiding this comment.
the cog components have baked-in color, so we would need to explicitly switch between them (blue, dark, light). the other bottom nav icons are also inline and simply inherit currentColor, so I thought it would be more consistent this way.
if this is about visual consistency: we could just use the existing cog svg inline as well
There was a problem hiding this comment.
correct we cannot use currentColor but it's still better to have 2 .svg files than having all icons in javscript bundle. That is the reason we add -light.svg and -dark.svg for all icons that need it.
There was a problem hiding this comment.
updated
note the other three menu icons are still inline JS
There was a problem hiding this comment.
ack. lets merge as is,.. one day I'll go over again and move them to svg files.
After moving Insurance to the Marketplace, "Settings" was now the only item inside "More", causing unnecessary friction to enter the actual settings on mobile. This moves settings directly into the menu bar, removing the redundant intermediate step.
72fb323 to
83c802b
Compare
2 participants
After moving Insurance to the Marketplace, "Settings" was now the only item inside "More", causing unnecessary friction to enter the actual settings on mobile.
This moves settings directly into the menu bar, removing the redundant intermediate step.