-
Notifications
You must be signed in to change notification settings - Fork 136
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
Add ability to disable registration of CRX layout and analytics settings #623
Add ability to disable registration of CRX layout and analytics settings #623
Conversation
to replace direct references of LayoutSettings and AnalyticsSettings
Thanks for doing this work. I think this implementation may be a bit over-engineered though. I don't plan on supporting having users create drop-in replacement settings models, as that could also get quite messy. My though would be a Django setting which simply does not register our settings models in the Wagtail Admin (UI hides them in the UI). However the models would still be intact as-is. As you've probably learned, too much stuff touches those models that the change of breakage becomes extremely high if someone customizes them. The assumption would be if you are disabling them in the Wagtail Admin, you'll need to customize the templates in which those settings are used, otherwise you'll end up getting the default values. |
I admit I was having fun and got carried away! It did exactly what I was hoping (and would not have affected existing installs) but I realize it was more than what you were expecting. A simple mechanism to disable the registration of the CRX models can also work for us. Just means a little more duplication on our side.
I started working on a template tag to replace the settings references in the templates but stopped short of going there! I've backed out everything except the decorator that conditionally will register the layout and analytics models. Let me know if this will work for you. |
@vsalvino - back to bump this. Is current PR acceptable or did you have another approach in mind? |
Thanks, this looks great! Appreciate the tests as well! I am going to do a bit of modification and split this setting into a couple individual settings. |
Building on the work from #623 See settings: * CRX_DISABLE_LAYOUT (not recommended) * CRX_DISABLE_ANALYTICS * CRX_DISABLE_NAVBAR * CRX_DISABLE_FOOTER The plan, based on what @jlchilders11 and I discussed, is to disable navbar and footer in the `pro` template, and provide a simple boilerplate navbar and footer for customization. Note: template filters `crx_settings` and `django_settings` have been removed, and replaced with tag ` {% django_setting "MY_SETTING" %}`. --------- Co-authored-by: Michael Steigman <msteigman@mgh.harvard.edu>
Allow developers who need to store additional site-specific settings and do not wish to have multiple "Settings" menus to disable the default models and create custom settings models.