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

Improve header 3 #605

Merged
merged 2 commits into from
Jun 2, 2022
Merged

Improve header 3 #605

merged 2 commits into from
Jun 2, 2022

Conversation

fairlighteth
Copy link
Contributor

Summary

@fairlighteth fairlighteth requested review from a team May 30, 2022 15:56
@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

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.

Nitpicks, otherwise all good

@@ -63,7 +63,7 @@ import { SwapVCowStatus } from 'state/cowToken/actions'
import LockedGnoVesting from './LockedGnoVesting'
import useBlockNumber from 'lib/hooks/useBlockNumber'
import usePrevious from 'hooks/usePrevious'
import { useCowFromLockedGnoBalances } from 'pages/Profile/LockedGnoVesting/hooks'
import { useCowFromLockedGnoBalances } from '@src/custom/pages/Account/LockedGnoVesting/hooks'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should the imports here (line 27 and 66) be from @srcpages/custom/pages/Account rather than pages/Account?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes you're right. I believe this has been fixed in improve-header-12 (last branch).

@@ -20,7 +20,7 @@ const PrivacyPolicy = lazy(() => import(/* webpackChunkName: "privacy_policy" */
const CookiePolicy = lazy(() => import(/* webpackChunkName: "cookie_policy" */ 'pages/CookiePolicy'))
const TermsAndConditions = lazy(() => import(/* webpackChunkName: "terms" */ 'pages/TermsAndConditions'))
const About = lazy(() => import(/* webpackChunkName: "about" */ 'pages/About'))
const Profile = lazy(() => import(/* webpackChunkName: "profile" */ 'pages/Profile'))
const Account = lazy(() => import(/* webpackChunkName: "profile" */ '@src/custom/pages/Account'))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
const Account = lazy(() => import(/* webpackChunkName: "profile" */ '@src/custom/pages/Account'))
const Account = lazy(() => import(/* webpackChunkName: "profile" */ 'pages/Account'))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is how it looks like in the last branch (improve-header-12)

const Account = lazy(() => import(/* webpackChunkName: "profile" */ '@src/custom/pages/Account'))

Let me know if that should be changed to your suggestion:

const Account = lazy(() => import(/* webpackChunkName: "profile" */ 'pages/Account'))

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah I see.
I'd that, unless it doesn't work without @src/custom, prefer the relative path

history.push('/profile' + location.search)
history.push('/account' + location.search)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd add a note here to update the path with @nenadV91 's changes where affiliate has it's own subpath now

<StyledNavLink to="/profile">Profile</StyledNavLink>
<StyledNavLink to="/account">Account</StyledNavLink>
<StyledNavLink to="/faq">FAQ</StyledNavLink>
<StyledNavLink to="/swap">More</StyledNavLink>
Copy link
Contributor

Choose a reason for hiding this comment

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

More goes to swap?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated in the last PR (#596)

Copy link
Contributor

@W3stside W3stside left a comment

Choose a reason for hiding this comment

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

Approving with small comment

* Switch to custom MenuDropdown compontent and create a POC.

* Improve header 5 (#603)

* Improve FAQ text

* Improve header 6 (#602)

* connect button styling

* Further flyout menu styling

* Improve header 7 (#601)

* Further flyout menu styling

* Uniform menu items styling

* Move styled components to styled

* Add hamburger icon with animation component.

* Improve header 8 (#600)

* Add mediaQuery hook

* Improve header 9 (#599)

* Progress mobile responsive menu

* Progress mobile responsive menu

* Improve header 10 (#598)

* Move to helper function

* Optimise menu code

* Improve header 11 (#597)

* Move URLS to an ENUM

* Fix enum issue.

* Styling and route fixes.

* Styling.

* Styling.

* Header/Menu improvement (waterfall PR FINAL)  (#596)

* Styling.

* Styling footer and bridge banner.

* Improve header 13 (#610)

* Fix network selector position.

* Fix exports/imports for network selector.

* Only open ordersPanel if account is true.
@fairlighteth fairlighteth merged commit a3d0e5c into improve-header-2 Jun 2, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Jun 2, 2022
@alfetopito alfetopito deleted the improve-header-3 branch June 3, 2022 07:38
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.

4 participants