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

Web spotlight root in Navigation #31

Merged
merged 5 commits into from
Jul 21, 2023

Conversation

Simply007
Copy link
Contributor

Motivation

Internal link: DEVRE-911

  • implement handling Web Spotlight root in navigation

Checklist

  • Code follows coding conventions held in this repo
  • Automated tests have been added
  • Tests are passing
  • Docs have been updated (if applicable)
  • Temporary settings (e.g. variables used during development and testing) have been reverted to defaults

How to test

Check the menu on "Ficto HealthText" after switching the "Solution" item of "Navigation" content type - dropdown should be resolved as expected.

@vercel
Copy link

vercel bot commented Jul 19, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
ficto-healthtech ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 21, 2023 11:00am
ficto-healthtech-imaging ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 21, 2023 11:00am
ficto-healthtech-surgical ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 21, 2023 11:00am

@Simply007
Copy link
Contributor Author

Env variables are only adjusted on Ficto Healthtech for specific branch (for collection mapping).

components/shared/ui/menu.tsx Outdated Show resolved Hide resolved
components/shared/ui/menu.tsx Outdated Show resolved Hide resolved
lib/constants/menu.ts Outdated Show resolved Hide resolved
lib/utils/env.ts Outdated Show resolved Hide resolved
@Simply007 Simply007 requested a review from JiriLojda July 21, 2023 11:02
Copy link
Member

@JiriLojda JiriLojda left a comment

Choose a reason for hiding this comment

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

The potential code simplification from removing the hook (and propagation from getStaticProps on every page) would be nice. However, we can wait for the rewrite to the app router which eliminates the need for manual propagation from getStaticProps. I'll leave it to you to decide.

lib/utils/env.ts Outdated Show resolved Hide resolved
@JiriLojda JiriLojda merged commit ff6b263 into main Jul 21, 2023
6 checks passed
@JiriLojda JiriLojda deleted the DEVREL-911/external-collections-link branch July 21, 2023 12:34
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.

2 participants