Skip to content

WD-30045 - Implement sliding navigation in snapcraft.io#5437

Merged
alvaromateo merged 8 commits intomainfrom
WD-30045
Nov 13, 2025
Merged

WD-30045 - Implement sliding navigation in snapcraft.io#5437
alvaromateo merged 8 commits intomainfrom
WD-30045

Conversation

@alvaromateo
Copy link
Copy Markdown
Contributor

@alvaromateo alvaromateo commented Oct 24, 2025

Done

  • Move changes from global-nav to snapcraft.io.

More specifically these changes include:

  • Changes to the header markup to make the sliding animations of dropdowns work in all screen sizes.
  • Necessary styles for the sliding animations.
  • JS listeners for handling all the clicks in the navigation links and menus.
  • Some patches to adapt the global-nav to the sliding style (mainly

How to QA

  • Open Demos and check that the navigation works the same as it does in Production.

Issue / Card

Fixes WD-30045

@webteam-app
Copy link
Copy Markdown

@steverydz steverydz self-requested a review October 24, 2025 16:11
@steverydz steverydz self-assigned this Oct 24, 2025
Comment thread entrypoint

if [ "${FLASK_DEBUG}" = true ] || [ "${FLASK_DEBUG}" = 1 ]; then
RUN_COMMAND="${RUN_COMMAND} --reload --log-level debug --timeout 9999"
RUN_COMMAND="${RUN_COMMAND} --logger-class canonicalwebteam.flask_base.log_utils.GunicornDevLogger"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Out of curiosity, why is this change needed?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It's optional, but given that you are already in the latest flask-base version, with this change you will get the pretty printed logs in DEV mode, which is a nice addition 😄

const topLevelNavigation = dropdownToggleButton.closest(".p-navigation__nav");
if (topLevelNavigation) {
const topLevelItems = topLevelNavigation.querySelectorAll(
":scope > .p-navigation__items",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice!

Copy link
Copy Markdown
Contributor

@steverydz steverydz left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR implements sliding navigation animations for snapcraft.io by migrating from a simpler global-nav implementation to a custom navigation system with enhanced interaction patterns. The changes introduce sliding animations for dropdown menus, improved mobile navigation handling, and better keyboard navigation support.

Key changes include:

  • Complete refactor of navigation JavaScript from a single global-nav.ts file to a modular structure with separate concerns (listeners, dropdown utilities, global nav patches)
  • Updates to HTML markup and SCSS to support sliding animations across all screen sizes
  • Upgrade of @canonical/global-nav package from 3.7.3 to 3.8.0

Reviewed Changes

Copilot reviewed 12 out of 13 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
templates/_header_macros.html Updates import path to new modular navigation structure
templates/_header.html Changes navigation wrapper classes to support sliding animations
static/sass/_snapcraft_p-navigation.scss Adds styles for sliding animations and wrapper positioning
static/js/base/navigation/login.ts Adds blank line (formatting only)
static/js/base/navigation/listeners.ts New file implementing all navigation interaction handlers
static/js/base/navigation/index.ts New entry point orchestrating navigation initialization
static/js/base/navigation/globalNav.ts New utility for patching global-nav mobile markup
static/js/base/navigation/dropdownUtils.ts New utility functions for dropdown state management
static/js/base/global-nav.ts Removed - functionality migrated to new modular structure
static/js/base/base.ts Removes unused navigation import
package.json Updates @canonical/global-nav to version 3.8.0
entrypoint Adds custom logger configuration for debug mode

Comment thread static/js/base/navigation/listeners.ts Outdated
Comment thread static/js/base/navigation/listeners.ts Outdated
Comment thread static/js/base/navigation/globalNav.ts Outdated
Comment thread static/js/base/navigation/globalNav.ts Outdated
Comment thread static/sass/_snapcraft_p-navigation.scss Outdated
Comment thread static/sass/_snapcraft_p-navigation.scss Outdated
Comment thread templates/_header.html Outdated
Comment thread static/js/base/navigation/globalNav.ts
@bartaz
Copy link
Copy Markdown
Member

bartaz commented Nov 6, 2025

image

On the /docs page (not available on demos) on mobile, there is excessive margin top between top nav and expanded navigation. On standard layout this margin accommodates for top nav height, but in docs layout it seems it's not needed. We may need to apply it only on standard layout.

@alvaromateo
Copy link
Copy Markdown
Contributor Author

image On the /docs page (not available on demos) on mobile, there is excessive margin top between top nav and expanded navigation. On standard layout this margin accommodates for top nav height, but in docs layout it seems it's not needed. We may need to apply it only on standard layout.

Good catch @bartaz! I just saw this bug is present also in PROD.
I've pushed changes that fix it and addressed all your other comments too.
Could you check these latest changes and approve if there's nothing else? Thanks!

@alvaromateo alvaromateo requested a review from bartaz November 12, 2025 10:23
Comment thread static/sass/_snapcraft_p-navigation.scss
Copy link
Copy Markdown
Member

@bartaz bartaz left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for the fixes.

@alvaromateo alvaromateo merged commit b60de79 into main Nov 13, 2025
13 checks passed
@alvaromateo alvaromateo deleted the WD-30045 branch November 13, 2025 13:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants