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

Allow specifying defaults for visible themes and widths #305

Merged
merged 8 commits into from
Feb 13, 2024

Conversation

nib-bturner
Copy link
Contributor

@nib-bturner nib-bturner commented Nov 27, 2023

This PR adds support for specifying default screen sizes and themes to be rendered when a state is not found in the query string or from being already specified in the StoreProvider react context.

Addresses: #176

Copy link

changeset-bot bot commented Nov 27, 2023

🦋 Changeset detected

Latest commit: ff0a443

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

This PR includes changesets to release 1 package
Name Type
playroom Minor

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

@nib-bturner nib-bturner marked this pull request as ready for review November 28, 2023 00:15
@nib-bturner nib-bturner requested a review from a team as a code owner November 28, 2023 00:15
@nib-bturner
Copy link
Contributor Author

Morning @askoufis,

Seems like there is quite some demand for the ability to specify a default theme and screen widths (eg #302). I am hoping you might be able to have a look over this PR, so we can start to use some of these features. Thanks!

@nib-bturner
Copy link
Contributor Author

@michaeltaranto any chance you can have a look over this one? We support about 20 different themes across ~6 different screen sizes. Initially loading all of these is very overwhelming and we would love to specifying defaults to prevent this cognitive overload.

Copy link
Contributor

@michaeltaranto michaeltaranto left a comment

Choose a reason for hiding this comment

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

Just a few tidy ups to called out. Thanks for this

Copy link
Contributor

@michaeltaranto michaeltaranto left a comment

Choose a reason for hiding this comment

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

Looks pretty clean mate. Thank you!

@michaeltaranto michaeltaranto merged commit ad60e01 into seek-oss:master Feb 13, 2024
2 checks passed
@nib-bturner nib-bturner deleted the feat/defaults branch February 13, 2024 03:55
@tob
Copy link

tob commented Apr 5, 2024

This feature is great, thanks for the PR, just a small note, it could be worth to mention in the readme that to the defaultVisibleThemes you don't pass the theme object but just the theme name.

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.

3 participants