- Notifications
You must be signed in to change notification settings - Fork8.7k
DEV: Migrate nav menu preferences to use formkit#36704
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Conversation
martin-brennan left a comment
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.
Just a minor change so far, but we do need a system spec here. Also you have some failing tests
| </template> | ||
| this.args.model | ||
| .save(this.saveAttrNames) | ||
| .catch(popupAjaxError) |
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 think you will need to handle rolling back the model values on error here
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 think .save() is what actually changes these items. I looked at the way other controllers for these preferences were doing it.
discourse/frontend/discourse/app/controllers/preferences/users.js
Lines 76 to 84 inee1a1c7
| @action | |
| save(){ | |
| this.set("saved",false); | |
| returnthis.model | |
| .save(this.saveAttrNames) | |
| .then(()=>this.set("saved",true)) | |
| .catch(popupAjaxError); | |
| } |
discourse/frontend/discourse/app/controllers/preferences/interface.js
Lines 381 to 469 inee1a1c7
| @action | |
| save(){ | |
| this.set("saved",false); | |
| constmakeThemeDefault=this.makeThemeDefault; | |
| if(makeThemeDefault){ | |
| this.set("model.user_option.theme_ids",[this.themeId]); | |
| } | |
| constmakeTextSizeDefault=this.makeTextSizeDefault; | |
| if(makeTextSizeDefault){ | |
| this.set("model.user_option.text_size",this.textSize); | |
| } | |
| if(!this.showColorSchemeSelector){ | |
| this.set("model.user_option.color_scheme_id",null); | |
| this.set("model.user_option.dark_scheme_id",null); | |
| }elseif(this.makeColorSchemeDefault){ | |
| this.set("model.user_option.color_scheme_id",this.selectedColorSchemeId); | |
| this.set( | |
| "model.user_option.dark_scheme_id", | |
| this.selectedDarkColorSchemeId | |
| ); | |
| if(this.selectedInterfaceColorModeId){ | |
| this.set( | |
| "model.user_option.interface_color_mode", | |
| this.selectedInterfaceColorModeId | |
| ); | |
| } | |
| } | |
| returnthis.model | |
| .save(this.saveAttrNames) | |
| .then(()=>{ | |
| this.set("saved",true); | |
| if(makeThemeDefault){ | |
| setLocalTheme([]); | |
| }else{ | |
| setLocalTheme( | |
| [this.themeId], | |
| this.get("model.user_option.theme_key_seq") | |
| ); | |
| } | |
| if(makeTextSizeDefault){ | |
| this.model.updateTextSizeCookie(null); | |
| }else{ | |
| this.model.updateTextSizeCookie(this.textSize); | |
| } | |
| if(this.makeColorSchemeDefault){ | |
| updateColorSchemeCookie(null); | |
| updateColorSchemeCookie(null,{dark:true}); | |
| }else{ | |
| updateColorSchemeCookie(this.selectedColorSchemeId); | |
| if( | |
| this.defaultDarkSchemeId>0&& | |
| this.selectedDarkColorSchemeId===this.defaultDarkSchemeId | |
| ){ | |
| updateColorSchemeCookie(null,{dark:true}); | |
| }else{ | |
| updateColorSchemeCookie(this.selectedDarkColorSchemeId,{ | |
| dark:true, | |
| }); | |
| } | |
| } | |
| if(this.selectedInterfaceColorModeId){ | |
| if(this.isViewingOwnProfile){ | |
| constmodeId=this.selectedInterfaceColorModeId; | |
| if(modeId===INTERFACE_COLOR_MODES.AUTO){ | |
| this.interfaceColor.useAutoMode(); | |
| }elseif(modeId===INTERFACE_COLOR_MODES.LIGHT){ | |
| this.interfaceColor.forceLightMode(); | |
| }elseif(modeId===INTERFACE_COLOR_MODES.DARK){ | |
| this.interfaceColor.forceDarkMode(); | |
| } | |
| } | |
| this.selectedInterfaceColorModeId=null; | |
| } | |
| this.homeChanged(); | |
| if(this.themeId&&this.themeId!==this.currentThemeId){ | |
| reload(); | |
| } | |
| }) | |
| .catch(popupAjaxError); | |
| } |
jordanvidrine commentedDec 16, 2025
I have added one, and removed a dated one that no longer seemed to be in use. |
Fairly straightforward refactor.