-
Notifications
You must be signed in to change notification settings - Fork 5
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
Wash list of theme names #226
Conversation
34c422e
to
e77e0c8
Compare
src/argus_htmx/themes/utils.py
Outdated
DATA_THEME_RE = f"\[data-theme={THEME_NAME_RE}\]" | ||
|
||
static_url = Path(settings.STATIC_URL).relative_to("/") | ||
stylesheet_path = static_url / default_htmx_settings.STYLESHEET_PATH |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this needs to look at the django.conf.settings since it might be overridden in another settings file (we do this)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall I like this.
There's one thing though. it seems that check_for_valid_themes_list
is scheduled to run after the Models are instantiated. which means that argus_htmx.themes.constants
is imported, which runs get_theme_names
, which throws the ImproperlyConfigured error and now we don't get our nice error message.
Also lolololol the check breaks the maybe it should be a warning instead? |
e4d5440
to
095ffa1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thoughts about using get_stylesheet_path()
in the error/warning messages?
Ensure that the themes shown in the theme-selector are actually installed.