-
Notifications
You must be signed in to change notification settings - Fork 135
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
Fix #701 Preferable default theme #746
Fix #701 Preferable default theme #746
Conversation
b9415ec
to
44504e4
Compare
Codecov Report
@@ Coverage Diff @@
## master #746 +/- ##
=======================================
Coverage 39.00% 39.00%
=======================================
Files 73 73
Lines 9699 9699
Branches 2229 2229
=======================================
Hits 3783 3783
- Misses 4754 4787 +33
+ Partials 1162 1129 -33 |
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.
I did a quick check to see if it could impact something else, but it looks like it is safe. Thanks for contributing!
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.
Woops, forgot to approve
Thank you for merging! |
Sadly a new user of mine still just ran into missing CSS on 0.5.0 :( |
@verymilan Can you re-save configuration again, please? |
I am not sure what you mean, but as usual, i have made them selecting a theme and save, which was fixing their issue as always |
Sorry for my poor English. What I did mean is... Can you have your users select "Default theme" and save it? It might be fix the issue? Under the hood, their current theme is saved as empty string |
Yes, i know, and this is how we did it - i was just |
Ah, sorry, I missed your point.
Yes, your realization is correct. |
This patch fixes #701 .
The issue occurs because form value of
email
is empty string""
when user selects "Default theme" and it is set as preferred theme name. There's no theme named""
, so any theme is not applied. By settingNULL
tousers.preferred_theme
when "Default theme" is selected, instance theme is used as the user's preferred theme.I used Diesel's
#[changeset_options(treat_none_as_null="true")]
attribute onUser
model. This may not be preferred butsave_changes
doesn't have option to setNULL
sometimes and keep current value otherwise(See http://diesel.rs/guides/all-about-updates/ . It says "Diesel doesn't currently provide a way to explicitly assign a field to its default value"). This is the reason why I used that attribute, but it may have too wide inpact. Could you consider?