B5 Responsive Forms #35519
B5 Responsive Forms #35519
Conversation
Make it easier to associate label with field on wide screens. Restore large-screen (lg) column widths used with Bootstrap 3. Tweak medium-screen (md) column widths to improve consistency.
Should reduce the overhead of future site-wide style changes such as when upgrading Bootstrap.
It is not longer required to update BooleanFields to use BootstrapCheckboxInput since responsive horizontal forms have been restored. Indeed, when the label value is changed to an empty string (label="") it adds awkward vertical space resulting in odd layout unless the form is also properly updated to use CheckboxField (in many cases this step is missed). BootstrapCheckboxInput may optionally be used if a (new) label value is added to the left, in addition to moving the previous label value to inline_label in the widget and updating the form layout to use CheckboxField. Otherwise the default BooleanField configuration is preferred. One form was converted back to the simpler version of BooleanField (as well as restoring proper alignment of form actions) as an example in this commit. Other instances exist and may be converted in the future.
There was a problem hiding this comment.
This is beautiful.
BootstrapCheckboxInput may optionally be used if a (new) label value is added to the left, in addition to moving the previous label value to inline_label in the widget and updating the form layout to use CheckboxField. Otherwise the default BooleanField configuration is preferred.
Is there any benefit to using BootstrapCheckboxInput? If there isn't a clear benefit, I think we should aim to deprecate it.
corehq/apps/hqwebapp/crispy.py
Outdated
| @@ -14,9 +14,9 @@ | |||
| from crispy_forms.utils import flatatt, get_template_pack, render_field | |||
|
|
|||
| CSS_LABEL_CLASS = 'col-xs-12 col-sm-4 col-md-4 col-lg-2' | |||
| CSS_LABEL_CLASS_BOOTSTRAP5 = 'col-xs-12 col-sm-4 col-md-3 col-lg-2' | |||
| CSS_LABEL_CLASS_BOOTSTRAP5 = 'field-label' | |||
There was a problem hiding this comment.
This is a major improvement.
| @@ -70,12 +69,9 @@ class ConnectionSettingsForm(forms.ModelForm): | |||
| required=False, | |||
| ) | |||
| skip_cert_verify = forms.BooleanField( | |||
| label="", | |||
| label=_('Skip certificate verification'), | |||
There was a problem hiding this comment.
Ah, this is so much nicer.
corehq/apps/hqwebapp/crispy.py
Outdated
| context.update({ | ||
| 'formactions': self, | ||
| 'fields_output': fields_html, | ||
| 'offsets': offsets, | ||
| 'offsets': '', |
There was a problem hiding this comment.
Nitpick: can offsets be removed altogether form this dict?
There was a problem hiding this comment.
Mabye, I'll take another look. I'm not sure how many places use (require?) it. It's hard to search for because it's a common word. I also need to check if crispy-boostrap3to5 needs it.
|
Coming back to this because I forgot to ask about testing. I see QA is TODO - what are you thinking there? I don't think we need to test every single form in HQ, but I do think it would be good to smoke test areas that use crispy heavily. Off the top of my head, messaging, users, locations, scheduled reports, and maybe project settings are the areas I think have the heaviest & most complex crispy usage. |
Yes. It's useful if you want a label on the left (
I agree, and that sounds like a good plan. |
| &:extend(.col-xs-12, .col-sm-8, .col-md-8, .col-lg-6); | ||
| } | ||
|
|
||
| :not(.field-label) + .field-control, |
Most HQ forms are designed to use a "horizontal" layout, saving vertical space on large screens. Despite the fact that the
form-horizontalclass was removed, horizontal forms are still a documented concept in Bootstrap 5. The label and field classes HQ code has always used in Bootstrap 3 and 5 are responsive, making form layout adjust automatically and comfortably to different screen sizes (although sometimes forms don't work well on small screens on Bootstrap 3 for other reasons).This PR introduces extension classes to encapsulate styling differences between Bootstrap 3 and 5, allowing code that uses them to remain unchanged during the transition. This makes the 3-to-5 migration simpler and easier because it requires less manual code changes. The style and migration guides have been updated for Bootstrap 5, including HTML and Crispy forms. Bootstrap 3 HTML form examples were not updated since those appear to be legacy. Hopefully we will not add many new B3 forms, but I am happy to update those examples as well if it would be useful.
Extensions classes could be used in other places as well. For example form row styles where
class="row mb-3"becomes something likeclass="form-row". This allows styling tweaks to be applied in one central place (the extension) rather than requiring widespread edits to HTML.Aside:
mb-3literally means margin bottom ..., which sounds a lot like an inline style. Would it not be better to encapsulate styling choices like that inside classes with semantic meaning rather than referencing cryptic abbreviated names that directly correspond to CSS properties?🐡 Review by commit.
[B5] Wide screen layout
[B5] Narrow screen layout
Safety Assurance
Safety story
Tested locally and on staging.
Automated test coverage
No.
QA Plan
https://dimagi.atlassian.net/browse/QA-7345
Rollback instructions