Skip to content

Commit

Permalink
fix: Changing color scheme resets session settings to defaults (#7407)
Browse files Browse the repository at this point in the history 
* Fix: incorrect saveing of color scheme in CMS.settings
* Add: changelog entry :-)
  • Loading branch information
@fsbraun
fsbraun committed Oct 14, 2022
1 parent e59c179 commit fcfe77f
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 1 deletion.
1 change: 1 addition & 0 deletions CHANGELOG.rst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ unreleased
* Add support for tel: and mailto: URIs in Advanced Page Settings redirect field.
* Make javascript dark mode functions available to popups as CMS.API.getColorScheme
and CMS.API.setColorScheme
* Fix bug where switching color scheme affects other settings
* Unlocalize page and node ids when rendering the page tree in the admin (#7175)
* Fixed permission denied error after page create (#6866)

Expand Down
7 changes: 6 additions & 1 deletion cms/static/cms/js/modules/cms.base.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -510,7 +510,12 @@ export const Helpers = {
setColorScheme: function (scheme) {
let body = $('html');

this.setSettings({ color_scheme: scheme });
if (!CMS.settings) {
// Settings loaded? If not, pls. load.
this.getSettings();
}
CMS.settings.color_scheme = scheme;
this.setSettings(CMS.settings);
if (scheme === 'auto') {
body.removeAttr('data-color-scheme');
body.find('div.cms iframe').each(function(i, e) {
Expand Down

2 comments on commit fcfe77f

@jrief
Copy link
Contributor

@jrief jrief commented on fcfe77f Oct 14, 2022

Choose a reason for hiding this comment

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

Hi @fsbraun , sorry for the delay.

I applied this patch to my CMS repository and rebundled everything. Unfortunately it does not have the desired effect.
Currently my workaround is to return immediately from this function:
https://github.com/jrief/django-cms/blob/df8d94156a80172e2b3d4d0872cd9a4cdf0ca3c7/cms/static/cms/js/modules/cms.toolbar.js#L809
which of course is a dirty hack.

@fsbraun
Copy link
Member Author

Choose a reason for hiding this comment

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

Hi @jrief ! Thanks for the update! That's very strange. I have explicitly tested for page tree settings to be preserved. Of course, you were right to run gulp bundle. As obvious as it is, I have found that sometimes I need to clear my browser cache forcefully to see changes of the new bundle taking effect (more often on Chrome than on Safari).

The less dirty way to patch is to just leave out the call to setSettings just in the line after your return. Also, you can set CMS_COLOR_SCHEME_TOGGLE to False in you settings.py to not show the color scheme toggle in the first place. Then setColorScheme is never called.

If have explicitly used CMS.settings.pagetree to test for other settings to be retained. Maybe I need more information: Would you have CMS.settings available before and after toggling the color scheme in the toolbar? What does CMS.API.Helpers.getSettings() return after the toggle (and is it different from CMS.settings)?

Please sign in to comment.