Skip to content
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

frontend: bottom nav for mobile #3211

Merged

Conversation

shonsirsha
Copy link
Collaborator

@shonsirsha shonsirsha commented Mar 4, 2025

Some screenshots for preview:
Screenshot 2025-03-18 at 10 05 04
Screenshot 2025-03-18 at 10 04 37
Screenshot 2025-03-18 at 10 03 49
Screenshot 2025-03-18 at 10 03 55

@shonsirsha shonsirsha marked this pull request as draft March 4, 2025 08:04
@shonsirsha shonsirsha force-pushed the frontend-mobile-bottom-menu branch from 5a8a461 to e2db87c Compare March 10, 2025 09:26
@shonsirsha shonsirsha changed the title WIP bottom menu frontend: bottom nav for mobile Mar 10, 2025
@shonsirsha shonsirsha force-pushed the frontend-mobile-bottom-menu branch from e2db87c to 47fc7a2 Compare March 10, 2025 09:48
</svg>
);

export const ExchangeIconSVG = () => (
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using "raw" svg instead of passing it into an image for 2 reasons:

  1. We can change its fill colour (for Link's active state) easily.
  2. Also prevents flickering which exists if we're using conditional rendering for svg that are passed into img. e.g
active ? <BlueIconImage /> : <DarkIconImage />

@shonsirsha shonsirsha marked this pull request as ready for review March 10, 2025 09:52
@shonsirsha shonsirsha force-pushed the frontend-mobile-bottom-menu branch from 47fc7a2 to 7cd2f7a Compare March 10, 2025 09:55
@shonsirsha shonsirsha requested a review from thisconnect March 10, 2025 10:02
@shonsirsha shonsirsha marked this pull request as draft March 10, 2025 11:46
@shonsirsha
Copy link
Collaborator Author

Converting back to draft as we need to figure out regarding the UI if there's no connected accounts available.

@shonsirsha shonsirsha force-pushed the frontend-mobile-bottom-menu branch from 7cd2f7a to 8a7f1e4 Compare March 17, 2025 08:48
@shonsirsha shonsirsha marked this pull request as ready for review March 17, 2025 08:52
@shonsirsha shonsirsha marked this pull request as draft March 17, 2025 09:02
@shonsirsha shonsirsha force-pushed the frontend-mobile-bottom-menu branch from 8a7f1e4 to 7810812 Compare March 18, 2025 09:02
@shonsirsha shonsirsha marked this pull request as ready for review March 18, 2025 09:03
Copy link
Collaborator

@thisconnect thisconnect left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tested in webdev only so far

@shonsirsha shonsirsha force-pushed the frontend-mobile-bottom-menu branch from 7810812 to 5c4d6a4 Compare March 24, 2025 05:36
@shonsirsha
Copy link
Collaborator Author

Thanks for the review @thisconnect , rebased & made changes PTAL 🙏

@shonsirsha shonsirsha force-pushed the frontend-mobile-bottom-menu branch 2 times, most recently from 989da64 to 1511cc2 Compare March 24, 2025 07:22
@shonsirsha
Copy link
Collaborator Author

shonsirsha commented Mar 24, 2025

@thisconnect @jadzeidan , I reduced the font size for the bottom menu label slightly:

Screenshot 2025-03-24 at 08 22 49

I think it looks better while saving space.

EDIT: also adjusted the height, sorry 😄

Screenshot 2025-03-24 at 08 39 16

@shonsirsha shonsirsha force-pushed the frontend-mobile-bottom-menu branch 2 times, most recently from e3e25e5 to 67d4c60 Compare March 24, 2025 07:54
Copy link
Collaborator

@thisconnect thisconnect left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just 2 small nits'

@shonsirsha shonsirsha force-pushed the frontend-mobile-bottom-menu branch 3 times, most recently from d9a6bdd to 67f831c Compare March 24, 2025 08:46
@shonsirsha
Copy link
Collaborator Author

@thisconnect , thanks! Changes addressed.

@shonsirsha shonsirsha force-pushed the frontend-mobile-bottom-menu branch from 67f831c to be7837d Compare March 24, 2025 10:00
Copy link
Collaborator

@thisconnect thisconnect left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you 👍

LGTM

created a bottom navigation bar and hide the sidebar on mobile
for better mobile UX.

Also created 2 new routes / page, the "all accounts" route and
"more" route to support this new bottom navigation bar.
@shonsirsha shonsirsha force-pushed the frontend-mobile-bottom-menu branch from be7837d to a6d6af0 Compare March 24, 2025 13:01
@shonsirsha shonsirsha merged commit 1b8d044 into BitBoxSwiss:master Mar 24, 2025
8 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants