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

[datetime] fix(DateInput): don't close when clicking time arrows #4240

Merged
merged 3 commits into from
Jul 28, 2020

Conversation

jez321
Copy link
Contributor

@jez321 jez321 commented Jul 23, 2020

Fixes #3474

Checklist

  • Includes tests
  • Update documentation

Changes proposed in this pull request:

  • Set tabIndex=-1 on the timepicker arrow span element to ensure e.relatedTarget on the FocusEvent passed to handlePopoverBlur in dateInput is valid, preventing inappropriate closing of the popover.

Added tests and checked in latest Chrome, Firefox and Safari.

Reviewers should focus on:

  • Whether the referenced bug has been fixed
  • Whether setting tabIndex=-1 has any other side effects

Screenshot

Before After
dpbefore dpfixed

@palantirtech
Copy link
Member

Thanks for your interest in palantir/blueprint, @jez321! Before we can accept your pull request, you need to sign our contributor license agreement - just visit https://cla.palantir.com/ and follow the instructions. Once you sign, I'll automatically update this pull request.

@jez321 jez321 marked this pull request as draft July 23, 2020 13:27
@jez321
Copy link
Contributor Author

jez321 commented Jul 23, 2020

After doing a bit more thinking I think it might be better to find a way to fix this that doesn't require as a big a change as changing the timepicker arrows from a span to button so I'll play around a bit more and see if I can come up with something.

@jez321
Copy link
Contributor Author

jez321 commented Jul 24, 2020

I've reverted the element type change and simply set tabIndex=-1 on the timepicker arrow span to ensure a valid relatedTarget is passed to focus events (specifically the FocusEvent passed to handlePopoverBlur in dateInput) which appears to fix the issue. Please let me know if this is acceptable or if you would like any changes to be made.

@jez321 jez321 marked this pull request as ready for review July 24, 2020 01:49
@adidahiya adidahiya self-requested a review July 24, 2020 14:50
Copy link
Contributor

@adidahiya adidahiya left a comment

Choose a reason for hiding this comment

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

great, thanks!

@adidahiya
Copy link
Contributor

adidahiya commented Jul 28, 2020

PR preview docs build

it's a little hard to test in the docs site. we should probably add this option to show time picker arrows on all the date input components, similar to DatePicker:

image

@adidahiya adidahiya changed the title Fix popover closing when clicking a timepicker arrow after selecting a month/year [datetime] fix(DateInput): don't close when clicking time arrows Jul 28, 2020
@adidahiya adidahiya merged commit 50720e3 into palantir:develop Jul 28, 2020
@IzaGz
Copy link

IzaGz commented Apr 2, 2021

I still have the problem on latest versions! ((( please help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DateInput popover closes when clicking TimePicker arrow after changing month
4 participants