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

refactor(storefront): Migrate page to new routing #2158

Merged
merged 2 commits into from
Jul 2, 2024

Conversation

poi33
Copy link
Collaborator

@poi33 poi33 commented Jun 25, 2024

Fix for #1748
Migrate all pages to app/ routing
Migrate some components to new routing structure

@poi33 poi33 requested review from mimarz and Barsnes as code owners June 25, 2024 17:37
@poi33 poi33 force-pushed the refactor/storefront-app-router branch from 995f006 to 60db4d5 Compare June 25, 2024 17:40
@poi33 poi33 changed the title Refactor(storefront): Migrate page to new routing refactor(storefront): Migrate page to new routing Jun 25, 2024
@poi33 poi33 force-pushed the refactor/storefront-app-router branch from 60db4d5 to 619d0e6 Compare June 25, 2024 17:43
Copy link
Member

@Barsnes Barsnes left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! ❤️
I looks like you forgot to rename the pages under grunnleggende
image

The GitHubLink.tsx link also seems to be throwing an error, but the path here would also need to be updated to route/page.mdx
image

We also need to add meta data, so we get titles for the pages.
This was previously done with a <Meta /> component, but in the app router we need to export it:

export const metadata = {
  title: '...',
  description: '...',
};

I'd like to move over to layout.tsx files as well, but it works now and can always be done later on. We also have the issue of props that layouts need 🤔

Otherwise everything looks great 🎯

Migrate all pages to app router
Migrate some components to new routing structure
Migrated all metadata to new export
Removed old Meta component
@poi33 poi33 force-pushed the refactor/storefront-app-router branch from 619d0e6 to 6c96429 Compare July 1, 2024 20:13
@poi33 poi33 requested a review from Barsnes July 1, 2024 20:15
Copy link
Member

@Barsnes Barsnes left a comment

Choose a reason for hiding this comment

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

Thanks a lot! ❤️
There are a few minor bugs when building, but nothing I can't fix after a merge.
The biggest part of moving over to the app router is done, and routing seems to be working perferctly!

I'll approve this and solve the remaining build errors in the next branch, thanks a lot! 😄
image

@Barsnes Barsnes merged commit 0d93712 into digdir:next Jul 2, 2024
0 of 3 checks passed
@poi33
Copy link
Collaborator Author

poi33 commented Jul 2, 2024

@Barsnes I had some trouble with the sizes. Tried to match it to the latest build off i think it was the react package. Tried to keep it in sync.

@poi33 poi33 deleted the refactor/storefront-app-router branch July 2, 2024 07:58
@Barsnes
Copy link
Member

Barsnes commented Jul 2, 2024

@Barsnes I had some trouble with the sizes. Tried to match it to the latest build off i think it was the react package. Tried to keep it in sync.

You might not have built it locally? We are using the workspace package, and not from npm.
A lot of good work either way!

mimarz pushed a commit that referenced this pull request Feb 21, 2025
resolves #1748
Migrate all pages to app/ routing
Migrate some components to new routing structure
@mimarz mimarz added the community-contribution-❤️ Pull request from developers outside the designsystemet team label Feb 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community-contribution-❤️ Pull request from developers outside the designsystemet team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants