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

[datetime2] feat(DateInput2): use Popover2, port DateInput impl #5440

Merged
merged 14 commits into from
Jul 19, 2022

Conversation

adidahiya
Copy link
Contributor

@adidahiya adidahiya commented Jul 13, 2022

Checklist

  • Includes tests
  • Update documentation

Changes proposed in this pull request:

Despite what the docs stated before this change, we were not using Popover2 properly in DateInput2. It was delegating to DateInput for the text input + popover implementation, which used Popover.

This PR ports over the remaining code from DateInput into DateInput2 and migrates to Popover2.

Reviewers should focus on:

  • No regressions from DateInput
  • Passing test suite

Screenshot

image

@blueprint-bot
Copy link

Port DateInput popover/input impl into DateInput2

Previews: documentation | landing | table | demo

@blueprint-bot
Copy link

fix DateInput2 tests

Previews: documentation | landing | table | demo

@blueprint-bot
Copy link

port DateInput tests, part 1

Previews: documentation | landing | table | demo

@blueprint-bot
Copy link

port DateInput tests, part 2

Previews: documentation | landing | table | demo

@blueprint-bot
Copy link

attempt to port one more test

Previews: documentation | landing | table | demo

@adidahiya adidahiya marked this pull request as ready for review July 18, 2022 16:28
@blueprint-bot
Copy link

port DateInput tests, part 4

Previews: documentation | landing | table | demo

@adidahiya
Copy link
Contributor Author

There's a bug in controlled mode -- the "Invalid date" message does not appear after typing in an invalid date:

DateInput2 DateInput
2022-07-18 12 39 34 2022-07-18 12 40 54

@blueprint-bot
Copy link

improve error states in controlled mode

Previews: documentation | landing | table | demo

@adidahiya
Copy link
Contributor Author

Things are much better now, but there is still this weird state you can get into when typing an incomplete date and the timezone is interpreted as a very unexpected offset:

image

Will fix that in a follow-up PR.

@blueprint-bot
Copy link

show error styling if message is present

Previews: documentation | landing | table | demo

@adidahiya adidahiya changed the title [datetime2] feat(DateInput2): use Popover2, port impl from DateInput [datetime2] feat(DateInput2): use Popover2, port DateInput impl Jul 19, 2022
@adidahiya adidahiya merged commit fbe3c9e into develop Jul 19, 2022
@adidahiya adidahiya deleted the ad/dateinput2-popover2 branch July 19, 2022 13:27
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.

2 participants