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

Add examples/theming: usage of multiple instances within different themes #2504

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

princerb
Copy link
Contributor

@princerb princerb commented Aug 9, 2024

. .
image image

image
image

@princerb
Copy link
Contributor Author

@samuelhwilliams Can this example be added?

@samuelhwilliams
Copy link
Contributor

samuelhwilliams commented Aug 13, 2024

I think this example is interesting - and my first reaction was "hey cool". But I don't know if we should merge this and implicitly say that this is a recommended/supported way to manage themes/swatches.

It feels very much like you're selecting a theme as a user, but you're changing it for the global flask admin app. I can imagine some confusion would arise from this in a multi-user environment? I suspect we'd need to think a bit harder about how to support this kind of theming in a multi-user way (and my focus probably won't be on adding support for that any time soon).

I think maybe an approach to take here is to improve the documentation around how our themes work, such that anyone reading that wanted to build this theme-switching functionality could do it - but that we aren't telling them how to do it so that they don't come to us expecting it to be a supported feature.

@princerb
Copy link
Contributor Author

princerb commented Aug 13, 2024

Sure, I also thought about it. Changing the global flask admin instance's value on load might be unsafe for some environments, but unless it is the theme's attribute, I directly did what I wanted.

But my main point was (I started this example when we had the idea of adding the new theme param) for those who can try and see all of the swatch themes in one example which includes fileadmin, different models.

The thing is here I think, if we talk about a multi-user environment (even if you want to support it or not), we just need to redefine and check the swatch param if it is callable. If callable for example, they can set their own function to it, like get_swatch_theme_from_current_user.

This could be an another modification inside theme module, but I haven't really thought if this would be very useful for the users. I mean, can they (admins of the site) somehow be willing mostly to use their own swatch themes by implementing it in this way?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants