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

Feat: Introduce Vertical Navigation #DS-1627 #1861

Merged
merged 6 commits into from
Jan 29, 2025

Conversation

crishpeen
Copy link
Member

@crishpeen crishpeen commented Jan 21, 2025

Description

  • Visual tests would fail, because this is only the web package implementation
  • What needs to be done in React:
    • update Drawer demo (remove helpertext)
    • add direction prop to Navigation with Direction dictionary and with horizontal as a default value
    • update docs (Navigation)
    • update tests (Navigation)
    • update demos (Navigation & UNSTABLE_Header)
    • update storybook (Navigation)

Additional context

Issue reference

@github-actions github-actions bot added the feature New feature or request label Jan 21, 2025
@crishpeen crishpeen changed the title Feat: Introduce Vertical Navigation #DS-1627 Feat(web): Introduce Vertical Navigation #DS-1627 Jan 21, 2025
Copy link

netlify bot commented Jan 21, 2025

Deploy Preview for spirit-design-system-storybook ready!

Name Link
🔨 Latest commit 155fa26
🔍 Latest deploy log https://app.netlify.com/sites/spirit-design-system-storybook/deploys/6798edbea1683d0008fbf9de
😎 Deploy Preview https://deploy-preview-1861--spirit-design-system-storybook.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Jan 21, 2025

Deploy Preview for spirit-design-system ready!

Name Link
🔨 Latest commit 155fa26
🔍 Latest deploy log https://app.netlify.com/sites/spirit-design-system/deploys/6798edbeb3e7970008b93789
😎 Deploy Preview https://deploy-preview-1861--spirit-design-system.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
1 paths audited
Performance: 96 (no change from production)
Accessibility: 91 (no change from production)
Best Practices: 100 (no change from production)
SEO: 91 (no change from production)
PWA: -
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify site configuration.

@crishpeen crishpeen force-pushed the feat/vertical-navigation branch from b2eebd0 to 7184850 Compare January 21, 2025 20:20
@literat
Copy link
Collaborator

literat commented Jan 22, 2025

What needs to be done in React:

  • update Drawer demo (remove helpertext)
  • add direction prop to Navigation with Direction dictionary and with horizontal as a default value
  • update docs (Navigation)
  • update tests (Navigation)
  • update demos (Navigation & UNSTABLE_Header)
  • update storybook (Navigation)

Did you add this to the appropriate issue or did you create a new one? If no, please do so. Thanks :-)

@literat
Copy link
Collaborator

literat commented Jan 22, 2025

Shouldn't the open menu item have also the hover state?

@crishpeen
Copy link
Member Author

What needs to be done in React:

  • update Drawer demo (remove helpertext)
  • add direction prop to Navigation with Direction dictionary and with horizontal as a default value
  • update docs (Navigation)
  • update tests (Navigation)
  • update demos (Navigation & UNSTABLE_Header)
  • update storybook (Navigation)

Did you add this to the appropriate issue or did you create a new one? If no, please do so. Thanks :-)

@curdaj is already working on it :) We will do it in one issue (as agreed on planning), but it will be a separate PR, so it is not too big.

@crishpeen crishpeen force-pushed the feat/vertical-navigation branch from 7184850 to d34b9f7 Compare January 23, 2025 14:08
Copy link
Contributor

@adamkudrna adamkudrna 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
Collaborator

@literat literat left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@crishpeen crishpeen force-pushed the feat/vertical-navigation branch 2 times, most recently from 20055d4 to c2483aa Compare January 28, 2025 11:57
@crishpeen crishpeen force-pushed the feat/vertical-navigation branch from c2483aa to 9d34fd1 Compare January 28, 2025 11:58
@crishpeen crishpeen force-pushed the feat/vertical-navigation branch from 9d34fd1 to 5bf6cf5 Compare January 28, 2025 12:01
@crishpeen crishpeen added the run-visual-tests Runs visual regression testing on this PR label Jan 28, 2025
@curdaj curdaj force-pushed the feat/vertical-navigation branch from 5f754f8 to 155fa26 Compare January 28, 2025 14:46
@curdaj curdaj changed the title Feat(web): Introduce Vertical Navigation #DS-1627 Feat: Introduce Vertical Navigation #DS-1627 Jan 28, 2025
@curdaj curdaj merged commit 6e23b99 into integration/header Jan 29, 2025
23 checks passed
@curdaj curdaj deleted the feat/vertical-navigation branch January 29, 2025 13:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request run-visual-tests Runs visual regression testing on this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants