canonical / canonical.com

Repository for the new version of canonical.com
Other
33 stars 66 forks source link

Implement the sliding navigation JS #1310

Closed petesfrench closed 1 month ago

petesfrench commented 2 months ago

Done

This work is based around the changes to found here

QA

Issue / Card

Fixes https://warthogs.atlassian.net/browse/WD-12081

webteam-app commented 2 months ago

Demo

Jenkins

demos.haus

petesfrench commented 2 months ago

@britneywwc re blackbox of doom: I have an issue here https://warthogs.atlassian.net/browse/WD-13674, but I think this should be dealt with on Vanilla.

Can you take another look please?

britneywwc commented 2 months ago

Thanks for addressing the comments. I found a few more bugs when re-reviewing.

When I navigate to Menu > Careers, there is weird padding on the header.

Screenshot 2024-07-26 at 5 34 02 PM

In the same navigation, there's also no way to scroll any lower to check out the links below "Diversity"

Screenshot 2024-07-26 at 5 35 25 PM 1
britneywwc commented 2 months ago

When I scroll all the way down in the navigation, it will continue the scroll on the main page - can we lock main page scroll when user is in mobile navigation view?

Another thing is when we reach the bottom on scroll, close the nav menu and reopen it. The nav menu reopens but it shows the bottom part of the navigation and might be misleading of a missing nav (screenshot below).

Screenshot 2024-07-26 at 5 40 01 PM
petesfrench commented 2 months ago

@britneywwc I spoke with Bartek about these issues, see conversation. They seem to be related to the Vanilla implementation of the sliding navigation, he is going to look into it earlier next week.

petesfrench commented 1 month ago

@britneywwc I would like to merge this so we can continue work on other aspects of the sliding navigation (such as the 'available roles'). I have created an issue to address this in the future: https://warthogs.atlassian.net/browse/WD-13674

britneywwc commented 1 month ago

Thanks for upstreaming this issue to Vanilla. If they are related to the sliding nav implementation then I'm okay to merge this 👍!