-
Notifications
You must be signed in to change notification settings - Fork 19
Move theme settings into fieldsets and containers #813
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
Conversation
localgov_base.theme
Outdated
| // Add settings in a details element for better organization | ||
| $form['localgov_base_settings'] = [ | ||
| '#type' => 'details', | ||
| '#title' => t('LocalGov Base Settings'), |
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.
This title get's inherited by the child themes. Should it change to the current theme name?
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.
Updated, good suggestion.
| // Create a fieldset for content status related settings | ||
| $form['localgov_base_settings']['content_status_fieldset'] = [ | ||
| '#type' => 'fieldset', | ||
| '#title' => t('Content Status Display'), |
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'm not sure about this heading , but I can't think of a better one! :)
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.
It's just a string. Let's leave it for now and we can easily change it later if we think of a better one.
|
@msayoung @tonypaulbarker This is ready for another review. |
ctorgalson
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.
👍
Closes #791
What does this change?
How to test
Change some settings, make sure they take effect and other settings do not break.
How can we measure success?
We have a tidier settings for for localgov_base and subthemes of it.
Have we considered potential risks?
Settings for current sites break (this should not happen).
Images