-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Align timepicker to calendar #7864
Conversation
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.
Works for me, just a note about using a variable if that width is related to anything else.
components/date-time/style.scss
Outdated
@@ -43,6 +43,7 @@ $datepicker__border-color: $light-gray-500; | |||
align-items: center; | |||
justify-content: center; | |||
margin-top: 10px; | |||
max-width: 248px; |
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.
Why was this size picked? Is it the max-width of the datepicker itself? If so, we should use a variable.
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 (manually calculated) width of the calendar itself, which doesn't seem to change at all. The number isn't used elsewhere, so whilst we could add a variable for it, I'm not sure we'd ever need/want to use it elsewhere.
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.
Makes sense; a comment explaining that would be great, in that case.
db1bbc5
to
0cc23c8
Compare
Description
At tablet-ish screen sizes, the calendar in the pre-publish panel is aligned to the left of the panel, whilst the timepicker attached to it floats way out in the middle of the screen, making them seem disconnected. Adding a max-width to the timepicker solves this issue and doesn't seem to break when using different languages, zoom levels, or font sizes.
Fixes #7863.
How has this been tested?
Tested in Chrome/Firefox on a Mac, in English, Hebrew, and Japanese (and at varying levels of zoom and font sizes).
Before:
After:
Types of changes
Simple bug fix.
Checklist: