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

[DST-379]: Docs: Persist Theme #3865

Merged
merged 23 commits into from
May 13, 2024
Merged

[DST-379]: Docs: Persist Theme #3865

merged 23 commits into from
May 13, 2024

Conversation

OsamaAbdellateef
Copy link
Contributor

No description provided.

Copy link

changeset-bot bot commented Apr 8, 2024

🦋 Changeset detected

Latest commit: 9b83a44

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 8 packages
Name Type
@marigold/docs Patch
@marigold/system Patch
@marigold/components Patch
@marigold/icons Patch
@marigold/theme-preset Patch
@marigold/theme-b2b Patch
@marigold/theme-core Patch
@marigold/storybook-config Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link

vercel bot commented Apr 8, 2024

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

Name Status Preview Comments Updated (UTC)
marigold-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 10, 2024 3:09pm
marigold-storybook ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 10, 2024 3:09pm
1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
marigold-production ⬜️ Ignored (Inspect) Visit Preview May 10, 2024 3:09pm

@sebald
Copy link
Member

sebald commented Apr 29, 2024

LGTM just one question: with this implementation, people can not deeplink (with a certain theme). Should we add this or leave it?

@sarahgm @OsamaAbdellateef @aromko @KadaStrophe

@OsamaAbdellateef
Copy link
Contributor Author

LGTM just one question: with this implementation, people can not deeplink (with a certain theme). Should we add this or leave it?

@sarahgm @OsamaAbdellateef @aromko @KadaStrophe

you mean when you have a deep link and then switch the theme u lose the deep link ? @sebald

docs/app/layout.tsx Outdated Show resolved Hide resolved
@sebald
Copy link
Member

sebald commented Apr 30, 2024

LGTM just one question: with this implementation, people can not deeplink (with a certain theme). Should we add this or leave it?
@sarahgm @OsamaAbdellateef @aromko @KadaStrophe

you mean when you have a deep link and then switch the theme u lose the deep link ? @sebald

Yes :) like always having the ?theme=core query appended

@OsamaAbdellateef
Copy link
Contributor Author

LGTM just one question: with this implementation, people can not deeplink (with a certain theme). Should we add this or leave it?
@sarahgm @OsamaAbdellateef @aromko @KadaStrophe

you mean when you have a deep link and then switch the theme u lose the deep link ? @sebald

Yes :) like always having the ?theme=core query appended

For me i would say it is good to have

@OsamaAbdellateef
Copy link
Contributor Author

LGTM just one question: with this implementation, people can not deeplink (with a certain theme). Should we add this or leave it?
@sarahgm @OsamaAbdellateef @aromko @KadaStrophe

you mean when you have a deep link and then switch the theme u lose the deep link ? @sebald

Yes :) like always having the ?theme=core query appended

done ✔️ @sebald

@sebald sebald self-requested a review April 30, 2024 12:04
aromko
aromko previously approved these changes May 3, 2024
@OsamaAbdellateef
Copy link
Contributor Author

Should be working now @sebald

@sebald sebald merged commit 5eafe50 into main May 13, 2024
12 checks passed
@sebald sebald deleted the presist-theme branch May 13, 2024 13:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:docs Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants