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

Wash list of theme names #226

Merged
merged 13 commits into from
Nov 14, 2024
Merged

Wash list of theme names #226

merged 13 commits into from
Nov 14, 2024

Conversation

hmpf
Copy link
Collaborator

@hmpf hmpf commented Nov 6, 2024

Ensure that the themes shown in the theme-selector are actually installed.

@hmpf hmpf added the bug Something isn't working label Nov 6, 2024
@hmpf hmpf self-assigned this Nov 6, 2024
src/argus_htmx/themes/utils.py Outdated Show resolved Hide resolved
src/argus_htmx/themes/utils.py Outdated Show resolved Hide resolved
@hmpf hmpf requested a review from elfjes November 6, 2024 13:51
@hmpf hmpf force-pushed the wash-themes-list branch 2 times, most recently from 34c422e to e77e0c8 Compare November 13, 2024 07:49
src/argus_htmx/settings.py Outdated Show resolved Hide resolved
src/argus_htmx/themes/utils.py Outdated Show resolved Hide resolved
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
Copy link
Contributor

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)

src/argus_htmx/themes/utils.py Outdated Show resolved Hide resolved
@hmpf hmpf requested a review from elfjes November 13, 2024 13:27
Copy link
Contributor

@elfjes elfjes left a 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.

src/argus_htmx/themes/utils.py Outdated Show resolved Hide resolved
src/argus_htmx/themes/utils.py Outdated Show resolved Hide resolved
src/argus_htmx/checks.py Outdated Show resolved Hide resolved
src/argus_htmx/checks.py Outdated Show resolved Hide resolved
src/argus_htmx/checks.py Outdated Show resolved Hide resolved
@elfjes
Copy link
Contributor

elfjes commented Nov 13, 2024

Also lolololol the check breaks the tailwind_config command so we can't regenerate 😂

maybe it should be a warning instead?

Copy link
Contributor

@elfjes elfjes left a 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?

src/argus_htmx/checks.py Show resolved Hide resolved
src/argus_htmx/themes/utils.py Outdated Show resolved Hide resolved
@hmpf hmpf merged commit 96c94d4 into Uninett:main Nov 14, 2024
4 checks passed
@hmpf hmpf deleted the wash-themes-list branch November 14, 2024 08:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants