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

feat: support theming solara #494

Merged
merged 9 commits into from
Feb 23, 2024

Conversation

iisakkirotko
Copy link
Collaborator

No description provided.

Copy link
Collaborator Author

iisakkirotko commented Feb 5, 2024

@iisakkirotko iisakkirotko force-pushed the 01-12-feat_task_and_use_task_for_better_asyncio_task_a_thread_execution branch from 06c4657 to de8a112 Compare February 5, 2024 14:56
@iisakkirotko iisakkirotko force-pushed the 02-05-feat_support_theming_solara branch from 0599961 to f8b0e84 Compare February 5, 2024 14:56
@iisakkirotko iisakkirotko force-pushed the 01-12-feat_task_and_use_task_for_better_asyncio_task_a_thread_execution branch from de8a112 to 27b701b Compare February 5, 2024 15:03
@iisakkirotko iisakkirotko force-pushed the 02-05-feat_support_theming_solara branch 3 times, most recently from b4d6684 to 10fe4e7 Compare February 8, 2024 16:09
@iisakkirotko iisakkirotko force-pushed the 01-12-feat_task_and_use_task_for_better_asyncio_task_a_thread_execution branch from 899a669 to 850c94b Compare February 8, 2024 16:14
@iisakkirotko iisakkirotko force-pushed the 02-05-feat_support_theming_solara branch 2 times, most recently from f58e32e to d03f038 Compare February 9, 2024 11:57
@maartenbreddels maartenbreddels force-pushed the 01-12-feat_task_and_use_task_for_better_asyncio_task_a_thread_execution branch 6 times, most recently from 3cdce2b to 1f4fc86 Compare February 12, 2024 21:05
Copy link
Contributor

@maartenbreddels maartenbreddels left a comment

Choose a reason for hiding this comment

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

I think this is going to be super cool!
Mostly worried about the naming, and maybe the theming.vue can be cleaned up a bit by using computed properties instead of using the clicks directly (difficult to follow).
(e.g. using a .is_auto() { return ...} )

solara/server/templates/solara.html.j2 Outdated Show resolved Hide resolved
solara/website/pages/api/theming.py Show resolved Hide resolved
solara/lab/components/theming.vue Outdated Show resolved Hide resolved
solara/server/app.py Show resolved Hide resolved
@maartenbreddels maartenbreddels force-pushed the 01-12-feat_task_and_use_task_for_better_asyncio_task_a_thread_execution branch 5 times, most recently from 3cea2ab to fcea18f Compare February 15, 2024 15:05
@maartenbreddels maartenbreddels force-pushed the 01-12-feat_task_and_use_task_for_better_asyncio_task_a_thread_execution branch 4 times, most recently from f981cfd to a9f836f Compare February 16, 2024 13:34
@iisakkirotko iisakkirotko force-pushed the 02-05-feat_support_theming_solara branch from f9b54bf to 84883d7 Compare February 16, 2024 15:25
@iisakkirotko iisakkirotko changed the base branch from 01-12-feat_task_and_use_task_for_better_asyncio_task_a_thread_execution to master February 16, 2024 15:25
@iisakkirotko iisakkirotko force-pushed the 02-05-feat_support_theming_solara branch 2 times, most recently from c41779e to 162cf58 Compare February 21, 2024 08:11
@iisakkirotko iisakkirotko marked this pull request as ready for review February 21, 2024 08:13
Copy link
Contributor

@maartenbreddels maartenbreddels left a comment

Choose a reason for hiding this comment

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

Looking good.
I think we should remove totally the variant_user_selectable and change

class ThemeSettings(BaseSettings):
    variant: ThemeVariant = ThemeVariant.light

to

class ThemeSettings(BaseSettings):
    dark: ThemeVariant = ThemeVariant.light

To make it backward compatible, we should check if SOLARA_THEME_VARIANT is set, and make theme.dark reflect that.

Also, think about how this plays with the cmd line arg "--theme-variant. Not sure how we should expose it with the rename, --dark=dark seems a bit odd.
This could be a reason to keep the name variant, let me know what you think

solara/lab/components/theming.vue Show resolved Hide resolved
solara/lab/components/theming.vue Show resolved Hide resolved
solara/server/templates/solara.html.j2 Show resolved Hide resolved
solara/server/templates/solara.html.j2 Outdated Show resolved Hide resolved
solara/server/templates/solara.html.j2 Outdated Show resolved Hide resolved
solara/lab/components/theming.py Outdated Show resolved Hide resolved
solara/lab/components/theming.py Outdated Show resolved Hide resolved
solara/server/server.py Outdated Show resolved Hide resolved
@iisakkirotko iisakkirotko force-pushed the 02-05-feat_support_theming_solara branch 3 times, most recently from 5456d37 to 82e45dc Compare February 23, 2024 15:07
@maartenbreddels maartenbreddels merged commit 238e09a into master Feb 23, 2024
25 checks passed
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.

None yet

2 participants