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

Nav footer redesign #409

Closed
wants to merge 20 commits into from
Closed

Nav footer redesign #409

wants to merge 20 commits into from

Conversation

henri-egger
Copy link
Contributor

@gappc

Here the PR for the new navigation and footer design.

Implementation Summary:

  • Everything follows the figma design.
  • The design changes to the mobile design at the lg breakpoint.
  • I changed how the navbar is overlaid above the rest of the page in AppLayout.vue to simplify the implementation of the dark overlay which appears when the mobile menu is opened.
  • I made sure the design works when logged in.
  • All text content is located in the locales/en.json file.

I will add the logic for te badge displaying the current environment once discussed here.

@henri-egger henri-egger requested a review from gappc July 14, 2023 15:07
@gappc
Copy link
Collaborator

gappc commented Jul 16, 2023

@henri-egger nice work, thank you for this PR 👍

There are some small issues and since the PR touches the base layout, I think it would be best if we could have a meeting where we discuss the stuff. That way it's easier for me to explain some of the layout concepts and desired behavior.

Maybe that could also be a chance that we can work out together some formalization of the layout and discuss other visualization issues e.g. as you already pointed out here #401 and here: #204 (comment)

@henri-egger
Copy link
Contributor Author

@gappc

Good idea, will you organize the meeting?

@henri-egger
Copy link
Contributor Author

@gappc all changes we discussed today are now implemented, what's missing is:

  • The badge
  • A solution for the spacing around the navbar (needs discussion with design team)
  • A solution for z-indices

I tried implementing the overlay using as little logic as possible, please look at it's implementation and tell me what you think.

</footer>
</ContentAlignmentX>
|
<ExternalLink href="/" tone="text">Privacy</ExternalLink>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Duplicate link to privacy?

@gappc
Copy link
Collaborator

gappc commented Jul 27, 2023

@henri-egger really nice work, thank you to put such effort in the improvements and optimizations 👍

While this PR is already in good shape and almost ready to be merged, there remain some small issues:

  • duplicate privacy link in the footer
  • with a width of 768px - 1023px the header menu icon (hamburger-menu) is positioned on the wrong side (see first image below)
  • the header looks odd when shown on Table-/Detail-/...Views (see second image below) -> I think we need to discuss this with the designers

Please note, that I will not be able to merge this PR during this week (I know it's your last week in this project at least for a while). There will be a productive deployment next Tuesday and I think it is better to give the changes in this PR some time in TEST environment before they are deployed to PROD.

(first image)
image

(second image)
image

@henri-egger
Copy link
Contributor Author

@gappc thanks ;).

I will fix issues 1 and 2, issue 3 is the thing we already talked about in the meeting. I think as well that it will be best to ask the designers to come up with a solution which matteo can then implement.

No problem regarding the release. Also, before this PR goes into prod, there needs to be a solution for z-indicies as it currently looks terrible on mobile when in table view with menu open. You might want to take a look at issue #401.

@henri-egger
Copy link
Contributor Author

@gappc I think everything on my part should be ok now.

Matteo Biasi and others added 3 commits August 22, 2023 17:23
feat: added Testing page in header trough VITE_APP_ENV_BADGE env config; optimized header appearance
@sseppi
Copy link
Contributor

sseppi commented Sep 19, 2023

@gappc we reviewed the code included in this PR and now it seems that everything is OK, so we can merge this PR in the development branch to put all changes officially in testing.

@mrabans
Copy link

mrabans commented Oct 10, 2023

@gappc you can merge this

@gappc
Copy link
Collaborator

gappc commented Oct 12, 2023

Closing as rebased PR was merged in #429

@gappc gappc closed this Oct 12, 2023
@gappc gappc deleted the nav-footer-redesign branch October 12, 2023 10:40
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.

4 participants