Skip to content
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

Remove leading zero span #319

Closed
wants to merge 2 commits into from
Closed

Remove leading zero span #319

wants to merge 2 commits into from

Conversation

vicke4
Copy link

@vicke4 vicke4 commented Nov 18, 2020

Hi @wojtekmaj,

First of all thanks for all the wonderful libraries you have built. This PR solves a problem I came across when I was working with react-daterange-picker.

I had to customize the style of the calendar for my needs. So, I added some custom styles for the inputGroup. That clashed with the style you have to manage the leading zeros (For your reference I've attached a GIF below).

This PR eliminates the issue. Let me know if I'm wrong somewhere or I have to change something in the PR.

react date picker leading zero span element problem

@@ -21,50 +21,6 @@ describe('DayInput', () => {
expect(input).toHaveLength(1);
});

it('renders "0" given showLeadingZeros if day is <10', () => {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As the span element is removed, I believe these tests are not needed anymore.

@wojtekmaj
Copy link
Owner

wojtekmaj commented Nov 20, 2020

Hi @vicke4 - thank you for this PR, but unfortunately I won't be able to accept it. The solution you've build has been proposed and considered many times before but is only compatible with Chromium, leading to leading zero issues on non-Chromium browsers.

Inputs are going to get a revamp though soon in #272, and perhaps then what you're suggesting will be possible - stay tuned.

@wojtekmaj wojtekmaj closed this Nov 20, 2020
@wojtekmaj wojtekmaj added the enhancement New feature or request label Nov 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants