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

Spike: Layout/breadcrumbs in Pages router #144

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

7emansell
Copy link
Collaborator

Ticket:

Investigates DR-2966.

This PR does the following:

  • Creates collections and divisions directory structure
  • Adds Layout component that contains header, DS provider, feedback, etc
  • Adds Layout to all pages
  • Creates a breadcrumbs context provider
  • Adds breadcrumbs context to /divisions/[slug], /collections/[slug], /divisions, /collections

How has this been tested?

Accessibility concerns or updates

Checklist:

  • I have added relevant accessibility documentation for this pull request.
  • All new and existing tests passed.

Copy link

vercel bot commented May 20, 2024

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

Name Status Preview Comments Updated (UTC)
digital-collections ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 23, 2024 6:26pm

);
};

// Imagine an API call somewhere in here
Copy link
Member

Choose a reason for hiding this comment

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

Have you tried passing this mocked API-fetched data to the component? How would this change if you use this breadcrumbs array as a prop in Collection, i.e. const Collection = ({ breadcrumbs }) => {.....

You could use the last object in the array as the value that is passed to setBreadcrumbs.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Right, sorry for the confusion of trying to represent two approaches within the same file but yes imagining that as a prop to Collections. If that's the case though, I wouldn't use the context at all

@7emansell 7emansell requested a review from avertrees May 21, 2024 14:03
@7emansell 7emansell changed the title Spike: Breadcrumbs and Layout in pages router Spike: Layout/breadcrumbs in Pages router May 21, 2024
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.

None yet

2 participants