Summary: | Space between minute and meridiem fields in time inputs is too large | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Aditya Keerthi <akeerthi> | ||||||
Component: | Forms | Assignee: | Aditya Keerthi <akeerthi> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | cdumez, changseok, esprehn+autocc, ews-watchlist, gyuyoung.kim, hi, webkit-bug-importer, wenson_hsieh | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | Safari Technology Preview | ||||||||
Hardware: | Mac | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Aditya Keerthi
2020-11-20 12:01:26 PST
Created attachment 414711 [details]
Patch
Comment on attachment 414711 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=414711&action=review r=me > Source/WebCore/html/shadow/DateTimeEditElement.cpp:174 > + // If the literal begins/ends with a space, the gap between two fields will appear Do we care about any other forms of whitespace? > Source/WebCore/html/shadow/DateTimeEditElement.cpp:178 > + // together by applying a negative margin. Is it possible to avoid using a negative margin? What about if we only applied the `padding` if the first/last character is not a space? > Source/WebCore/html/shadow/DateTimeEditElement.cpp:179 > + if (text.characterAt(0) == ' ') NIT: `text.startsWith(' ')` > Source/WebCore/html/shadow/DateTimeEditElement.cpp:180 > + element->setInlineStyleProperty(CSSPropertyMarginLeft, -1, CSSUnitType::CSS_PX); What about RTL? Should this be `CSSPropertyMarginInlineStart`? > Source/WebCore/html/shadow/DateTimeEditElement.cpp:181 > + if (text.characterAt(text.length() - 1) == ' ') NIT: `text.endsWith(' ')` > Source/WebCore/html/shadow/DateTimeEditElement.cpp:182 > + element->setInlineStyleProperty(CSSPropertyMarginRight, -1, CSSUnitType::CSS_PX); ditto (:180) (In reply to Devin Rousso from comment #3) > Comment on attachment 414711 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=414711&action=review > > r=me > > > Source/WebCore/html/shadow/DateTimeEditElement.cpp:174 > > + // If the literal begins/ends with a space, the gap between two fields will appear > > Do we care about any other forms of whitespace? We don't because other forms of whitespace should not appear in a date/time format string. > > Source/WebCore/html/shadow/DateTimeEditElement.cpp:178 > > + // together by applying a negative margin. > > Is it possible to avoid using a negative margin? What about if we only > applied the `padding` if the first/last character is not a space? If we don't add the padding, the selected appearance of the fields would appear too squished, and would also be inconsistent with the other fields. The only other alternative I can think of to avoid using a negative margin would be to replace all spaces with fixed-width spans, but that would be a much more complex solution. > > Source/WebCore/html/shadow/DateTimeEditElement.cpp:180 > > + element->setInlineStyleProperty(CSSPropertyMarginLeft, -1, CSSUnitType::CSS_PX); > > What about RTL? Should this be `CSSPropertyMarginInlineStart`? Will change to use `CSSPropertyMarginInlineStart` and `CSSPropertyMarginInlineEnd`. Created attachment 414751 [details]
Patch for landing
Committed r270148: <https://trac.webkit.org/changeset/270148> All reviewed patches have been landed. Closing bug and clearing flags on attachment 414751 [details]. |