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

Header/Menu improvement (waterfall PR FINAL) #596

Merged
merged 3 commits into from
Jun 2, 2022

Conversation

fairlighteth
Copy link
Contributor

@fairlighteth fairlighteth commented May 30, 2022

Summary

This branch improve-header-12 is the last in the waterfall PR to improve the header/menu for all viewports:

  • Addresses issue Create new Menu / Header #570
  • Removes decimals for the shown COW balance
  • For small/mobile devices, it shows a fixed footer menu, with the COW button and the connected account.
  • Introduces a drop down menu structure/component. Incl. mobile (hamburger) menu support.
Screen.Recording.2022-05-30.at.16.13.24.mov
Screen.Recording.2022-05-30.at.16.59.46.mov
Screen.Recording.2022-05-30.at.16.13.05.mov
Screen.Recording.2022-05-30.at.16.12.36.mov
Screen.Recording.2022-05-30.at.16.12.05.mov
Screen.Recording.2022-05-30.at.16.11.29.mov
Screen.Recording.2022-05-30.at.17.01.07.mov

Todo for future PRs:

  • Style the network selector for mobile/small viewports. Specifically to have it be an overlay menu covering ~70% of the viewport.
  • Appzi button probably should not overlay the mobile main menu.

@fairlighteth fairlighteth requested review from a team May 30, 2022 15:35
@github-actions
Copy link
Contributor

CLA Assistant Lite All Contributors have signed the CLA.

@github-actions
Copy link
Contributor

  • 🔭 GP Swap: CoW Protocol v2 Swap UI

@fairlighteth fairlighteth changed the title Header/Menu improvement (waterfall PR) Header/Menu improvement (waterfall PR FINAL) May 30, 2022
@fairlighteth
Copy link
Contributor Author

Got a failed cypress test:

/home/runner/work/cowswap/cowswap/cypress-custom/screenshots/swapMod.ts/Swap (mo (1920x1080)
d) -- can enter an amount into output

Copy link
Collaborator

@alfetopito alfetopito left a comment

Choose a reason for hiding this comment

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

Small issue with desktop when the network menu is collapsed

Screen.Recording.2022-05-31.at.10.31.15.mov

Also, isn't the threshold for collapsing the menu too harsh?

Screen Shot 2022-05-31 at 10 36 56
Screen Shot 2022-05-31 at 10 37 04

There's plenty of space to show it with that size IMO

@fairlighteth
Copy link
Contributor Author

Small issue with desktop when the network menu is collapsed

Good find there. I think I'll apply a simple fix for now, until I make it an actual 'mobile' like menu.

There's plenty of space to show it with that size IMO

In this state you showed me, yes there's space. But a few things to consider:

  • In the future the menu can grow in number of items
  • Your (v)COW balance here is 0, but theoretically could have multiple digits.
  • Your connected wallet could have a longer ENS name
  • Your connect wallet ETH balance could have multiple digits

All of this can be optimized for individually (eg ellipsis or number abbreviation), but keeping a safe margin here seems sensible from that point of view.

* Fix network selector position.

* Fix exports/imports for network selector.

* Only open ordersPanel if account is true.
@fairlighteth fairlighteth merged commit 16b304b into improve-header-11 Jun 2, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Jun 2, 2022
@alfetopito alfetopito deleted the improve-header-12 branch June 3, 2022 07:36
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants