-
-
Notifications
You must be signed in to change notification settings - Fork 8.9k
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 alignment of settings widgets #3859
Conversation
The problem here is that very high fields will have their label centered vertically, way down. What happens in the example once you have 20 API tokens? |
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.
LGTM
+) the text area is closer to the sliding stuff at its bottom So it's a -1 from me. |
@Wadeck if you vote -1, could you please request changes next time so that it does not get missed? |
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.
See comments above
Please proceed! I am opposed to the vertical middle alignment when we are using the two columns "design", because it's bad for the reader "flow". As you say, the edge cases are perhaps not numerous, but I just don't understand the goal to do something that is not better for common cases and worse for some corner cases. If the only purpose is to have the labels on the left aligned with the middle of the one-liner inputs, in that case, I can only propose to add a padding top so that the alignment is good for 95% of the cases and for text-area, there is no issue. |
Regarding your response to,
I'd like to note that you start the page at the top. In a vertically long section as the one with the API tokens, it'd take some additional scrolling to actually see the vertically centered label when you start from the top. That seems like a bigger deal than not seeing it when further down -- you've already seen it. |
Seems like a reasonable idea (a lot of space is wasted due to -- sometimes nested -- label columns), although you should be prepared to encounter massive problems in this endeavor. We can't really overhaul the form structure of nested tables due to https://wiki.jenkins.io/display/JENKINS/Structured+Form+Submission, and plugins sometimes build their own UI elements with the assumption that the structure looks like it currently does. While it should be relatively possible to change the core UI, Jenkins will need to continue working reasonably well with most plugins, and certainly all popular plugins. This is easily JEP/SIG levels of difficulty and effort. |
This is an independent change. I'm on vacation this week. |
@jsoref would you have time to work on this? |
@alecharp: I'm not sure what people want me to do. |
I'm also -1 on the middle align. @jsoref I suggest bringing this up in a UX SIG meeting so that this type of work can be coordinated better. |
I'm with @Wadeck here, I see more natural having it at the top, to see it rapidly. I think this change is controversial, maybe not having the left column is better but it's a bigger change. So my proposal is to close this PR. |
I think there is no consensus here, so I would recommend you to open a proposal in the mailing list, @jsoref. If you agree with that I will change the Here my opinion as well:
|
At some point, the default layout for various widgets input/textarea/label may have changed a bit. But at least in modern time, the behavior of input-type=checkbox w/ label as rendered w/ the jenkins styles is pretty bad, and the use of td{vertical-align:top} makes for a pretty ugly outcome when a setting interacts with an input.
Before
After
Proposed changelog entries
Submitter checklist
* Use the
Internal:
prefix if the change has no user-visible impact (API, test frameworks, etc.)Desired reviewers