Claro datetime range min/max-width

Created on 30 November 2021, over 2 years ago
Updated 21 February 2024, 4 months ago

Problem/Motivation

Claro datetime range min-width is causing "empty" fields (containing dd/mm/yyyy placeholder) to be wider than filled fields. See image.
This will only occur when viewing the form for saved content and one date field has a date set and the other does not. If the value is one you just added and haven't yet saved, the widths will still be the same.

Steps to reproduce

  1. Set up a fresh install of (the latest) Drupal 10.x
  2. Enable the Datetime Range core module in /admin/modules
  3. Add a new field of type 'Date range' to a new or existing content type
  4. Create a new node of the content type you just added your field on via /node/add
  5. Before you fill out the Date range field on the /node/add/xxx form, it seems to look fine
  6. Fill out all fields of the node, except the end date/time and try to save
  7. Drupal will throw a form validation error, asking you to choose an end date
  8. As you can see in the following screenshots, the empty date field is wider than the other:

Chrome 113 on MacOS:

Firefox 112 on MacOS: Not an issue, this is the expected behavior of characters in non-monospaced fonts not all having the same width.

Proposed resolution

Option 1:
Add a max width in CSS to prevent the empty date input field to exceed a filled out date field.
@bnjmnm: "This is not ideal as different languages or accessibility needs can result in fonts/characters that occupy a different amount of space."
@bnjmnm: "The problem is extra unwanted margin added in some instances. That extra margin should be fixed instead of bloating the size of other datepickers to match it for the sake of symmetry."

Option 2:
@bnjmnm: "The difference is the spacing around the calendar picker icon - so that icon is what should be targeted in your solution."

Option 3:
???

Remaining tasks

  • Decide on the optimal solution
  • Create a patch
  • Test and review patch
  • Commit!

User interface changes

None.

🐛 Bug report
Status

Needs work

Version

11.0 🔥

Component
Claro 

Last updated about 10 hours ago

Created by

🇳🇱Netherlands Drumanuel

Live updates comments and jobs are added and updated live.
  • Needs issue summary update

    Issue summaries save everyone time if they are kept up-to-date. See Update issue summary task instructions.

Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • Status changed to Needs work over 1 year ago
  • 🇺🇸United States smustgrave

    This issue is being reviewed by the kind folks in Slack, #needs-review-queue-initiative. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge request as a guide.

    This issue summary could be updated. What's the proposed solution, remaining tasks? Could use before/after screenshots since it's a UX bug.

    Did not test.

  • 🇺🇸United States bnjmnm Ann Arbor, MI

    Updated issue summary to point out that the issue can only be reproduced if viewing saved content. I was able to reproduce it. (also not sure it's something that needs to be changed, but that's just my take)

  • First commit to issue fork.
  • Open on Drupal.org →
    Environment: PHP 8.1 & MySQL 5.7
    last update about 1 year ago
    Not currently mergeable.
  • @dsandhya opened merge request.
  • 🇮🇳India dsandhya

    I am able to reproduce this issue with version 10.1.x-dev and I have created this patch it's working fine.

  • 🇺🇸United States bnjmnm Ann Arbor, MI

    All the solutions proposed so far include setting min and/or max width. This is not ideal as different languages or accessibility needs can result in fonts/characters that occupy a different amount of space. No more min-width/max-width solutions please 🙂

    You'll also notice that the dates themselves have the same width in each scenario. The difference is the spacing around the calendar picker icon - so that icon is what should be targeted in your solution.

    A quick check of firefox shows the reported issue isn't happening there, so this looks to be a webkit thing. Just target the calendar picker icon in webkit:

    [type="date"]::-webkit-calendar-picker-indicator {
        margin-left: -12px;
    }
  • First commit to issue fork.
  • Open in Jenkins → Open on Drupal.org →
    Environment: PHP 8.1 & MySQL 5.7
    last update about 1 year ago
    29,378 pass
  • @rpayanm opened merge request.
  • Open in Jenkins → Open on Drupal.org →
    Environment: PHP 8.1 & MySQL 5.7
    last update about 1 year ago
    Custom Commands Failed
  • 🇺🇾Uruguay rpayanm

    I applied the #29's suggestion.
    Please review.

  • 🇺🇸United States smustgrave

    Moving to NW for the issue summary update to include the proposed solution. As #29 pointed out seems to be different approaches.

  • 🇳🇱Netherlands tinto Amsterdam

    Cleaning up the issue summary by removing some irrelevant info and adding steps to reproduce, proposed solutions and other relevant info.

    Hope this makes it somewhat easier for others/maintainers to review this issue.

  • 🇳🇱Netherlands tinto Amsterdam

    Re #29: I do see the problem in Firefox (v112 on MacOS) too, albeit slightly more subtle. See the screenshot comparison in #34 and the issue summary.

  • 🇺🇸United States bnjmnm Ann Arbor, MI

    The space difference in Firefox screenshot looks like normal character width variation that is inherent to non-monospaced fonts. The “mm” being wider than “05” is expected behavior, not anything that should be addressed with a code change.

    And while the reported issue isn’t particularly consequential either, the margin difference with the indicator does not seem as-expected and adding css to address it would be justified.

  • 🇺🇸United States bnjmnm Ann Arbor, MI
  • 🇳🇱Netherlands tinto Amsterdam

    Applying a negative margin proposed in #29 does seem to do the trick. And I agree it's a better solution than increasing the min-width to an arbitrary value.

    Either way, I'm not really sure how I feel about having to solve what is basically a bug/quirk in Chrome. Are we okay with solving it this way - and perhaps having to roll back this styling if one day Chromium does fix this on their end?

    P.S. Here's a codepen to test this outside of Drupal: https://codepen.io/Tinto/pen/XWxZGOx.

  • 🇺🇸United States bnjmnm Ann Arbor, MI

    Either way, I'm not really sure how I feel about having to solve what is basically a bug/quirk in Chrome. Are we okay with solving it this way - and perhaps having to roll back this styling if one day Chromium does fix this on their end?

    That's a good point! There is precedence for making this kind of change to account for a browser quirk: We find (or potentially file) the issue to the issue tracker of the browser in question, then apply a fix with a @todo pointing to the browser issue and instructing to remove the fix when that issue is addressed. It usually takes a few years after the problem is addressed to remove the Drupal fix, but it's also not problem causing code so it's fine having it hang around.

    However I should also mention that any time I've seen this approach it has been for an issue that is a little more consequential than the one reported here, but if someone were to track down the info needed to make an appropriate @todo, I'd still be fine RTBCing or committing.

  • 🇳🇱Netherlands tinto Amsterdam

    Interesting! The only related chromium bug I could find is this one: https://bugs.chromium.org/p/chromium/issues/detail?id=1090500. It was fixed almost three years ago and does not seem to cover our issue 100%.

    One of the comments suggests limiting the whitespace by using margin-inline-start: 0 on the calendar picker pseudo element. Setting this value to zero does not fix our issue, but setting a negative value does. It renders pretty much the same result as #29.

    My proposal is to set a negative margin of 0.875em (14/16), so that it plays well in case the font-size is overridden for some reason. So looking at the current code, I would add this to /core/themes/claro/css/components/form--text.css:

    .form-element--type-date::-webkit-calendar-picker-indicator {
      margin-inline-start: -0.875em;
    }
    

    If we can get consensus that this is the best possible fix, I can write a small patch.

    @bnjmnm: should we still open up a new issue for Chromium and add a comment in the patch that refers to that issue?

  • 🇮🇳India karanpagare

    Adding patch as per #40

Production build 0.69.0 2024