-
Notifications
You must be signed in to change notification settings - Fork 7
left align feedback form labels #2916
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
base: main
Are you sure you want to change the base?
Conversation
// !important is needed to override | ||
// https://github.com/sul-dlss/exhibits/blob/main/app/assets/stylesheets/modules/bootstrap_overrides.scss#L42-L46 | ||
#new_contact_form .mb-3 .col-form-label { | ||
text-align: left!important; |
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.
The whole point of dd52b2c was to align the form labels to the right. If we want them left aligned now, why not remove the rules in bootstrap_overrides.scss
?
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.
☝️ that looks like a bug to me (labels above and right aligned) and it's present in production right now too. Maybe it'll be easier to sort this out if we fix that bug first. cc @dbranchini @alundgard
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.
Per this original issue (#1549), I think it makes sense to right-align the labels when they appear in-line with the form field.
But if the labels appear above the form field, then it makes more sense to left-align them.
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.
@alundgard do we want some forms to have right aligned labels and some to be left aligned? That seems inconsistent.
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 the forms would have the same alignment, but the right-or-left alignment would be responsive to view width, if possible.
The right-alignment looks fine on wider views (when the labels are in-line with the fields), but right-alignment looks strange on narrower views (when the labels are above the fields).
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.
@dnoneill given what Alan has said, I think we ought to close this PR without merging.
closes #2737
Before:

After:

Before:

After: