-
-
Notifications
You must be signed in to change notification settings - Fork 825
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
Improve formatting for settings checkboxes #14419
Conversation
(Standard links)
|
@colemanw this looks good but I think the form validation stopped working (ideally would be a callback rather than in the form) |
settings/Core.setting.php
Outdated
'class' => 'crm-select2', | ||
], | ||
'quick_form_type' => 'CheckBoxes', | ||
'html_type' => 'crm-checkbox-list', |
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.
@colemanw better add this to the setting spec if we have a new type
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.
@eileenmcnaughton which spec is that?
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.
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.
Hmm, those docs suggest that html_type
is preferred over quickform_type
but when I was stepping through the code, html_type
was getting ignored by the buildForm fn, which is why I felt free to make up something new and stick it in there.
Buuuuut,
Maybe instead of making up something new, we should just apply this styling to all checkboxes on settings pages. It's nicer that what's there already. Grep shows seven settings other than this one of type checkboxes.
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.
Yeah - that makes sense. Probably we do prefer html_type since it makes more sense but the code couldn't get there all in one go...
f6c4294
to
bfd9c35
Compare
Wow this one is 50 points! |
@eileenmcnaughton as we discussed I've increased the scope of this to restyle all 7 settings fields of type "checkboxes". In the process I was able to gut the entirety of I fixed the form validation. I agree ideally it would be a callback in the settings metadata... but that mechanism doesn't provide any means of supplying a message to the user. |
@colemanw any ideas on how to make that a little prettier |
No I'm just resigned to the fact that quickform is ugly. |
ok |
Overview
Restyle the Components setting field. Previously this was an AdvMultiSelect element, then it was converted by #14393 to Select2, which still doesn't look quite right. This changes the style to a checkbox list which reuses the familiar row-highlight style.
Before #14393
After #14393
After this PR
Notes
This lays some groundwork for a simplification of #13565