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.

Sign up for GitHub

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

Jump to bottom

Decouple keyword time range validation from keyword time range preview in time range picker. #18879

Merged
merged 6 commits into from
Apr 24, 2024
Merged

Decouple keyword time range validation from keyword time range preview in time range picker. #18879

merged 6 commits into from
Apr 24, 2024

Conversation

linuspahl
Copy link
Contributor

@linuspahl linuspahl commented Apr 3, 2024 •
edited

Please note: This PR needs a backport to 6.0

Description

Motivation and Context

With this PR we are separating the time range validation from time range preview in time keyword tab of the range picker.

This approach has multiple advantages

  • We are no longer using the validate prop of the keyword formik field and have all the validation in the form validate function
  • We no longer need to maintain the validatingKeyword state to disable the submit button while the keyword is being validated
  • The keyword preview and validation logic is a lot simpler and less error-prone

The main disadvantage:

  • we are currently doing two requests to test the keyword time range in the date time picker, on change. One for the validation and one for the preview.

This restructuring is fixing three bugs:

  • the problem described in Search bar: Keyword time range time zone not set #18809. We are now always including the user time zone when defining a keyword time range.
  • before it often happened that the submit button stayed disabled after switching between the keyword tab and the other tabs
  • before this change the keyword time range preview was empty when opening the keyword tab while it contained the default keyword time range.

Fixes #18809

@linuspahl linuspahl marked this pull request as draft April 3, 2024 11:16
@linuspahl linuspahl marked this pull request as ready for review April 3, 2024 11:17
@linuspahl linuspahl force-pushed the fix/initial-keyword-time-range branch 3 times, most recently from 5d018da to 63c9230 Compare April 5, 2024 11:24
@linuspahl linuspahl changed the title Consider user time zone for initial keyword time range. Decouple keyword time range validation from keyword time range preview in time range picker. Apr 5, 2024
@linuspahl linuspahl force-pushed the fix/initial-keyword-time-range branch from 63c9230 to 0972600 Compare April 17, 2024 08:54
@linuspahl linuspahl force-pushed the fix/initial-keyword-time-range branch from 0972600 to be78a90 Compare April 19, 2024 09:20
@linuspahl linuspahl merged commit 9c5a969 into master Apr 24, 2024
6 checks passed
@linuspahl linuspahl deleted the fix/initial-keyword-time-range branch April 24, 2024 08:24
linuspahl added a commit that referenced this pull request Apr 24, 2024
@linuspahl
Decouple keyword time range validation from keyword time range previe…
83c8317 
…w in time range picker. (#18879)

* Consider user time zone for initial keyword time range.

* Make sure to not disable submit button when chanign tabs.

* Decouple keyword time range validation from keyword time range preview in time range picker.

* Adding changelog.

* Fixing test
@linuspahl linuspahl mentioned this pull request Apr 24, 2024
Merged
linuspahl added a commit that referenced this pull request May 8, 2024
@linuspahl
Decouple keyword time range validation from keyword time range previe…
2e93782 
…w in time range picker. (#18879)

* Consider user time zone for initial keyword time range.

* Make sure to not disable submit button when chanign tabs.

* Decouple keyword time range validation from keyword time range preview in time range picker.

* Adding changelog.

* Fixing test
dennisoelkers pushed a commit that referenced this pull request May 13, 2024
@linuspahl
Decouple keyword time range validation from keyword time range previe…
31a2d5f 
…w in time range picker. (#18879) (#19155)

* Consider user time zone for initial keyword time range.

* Make sure to not disable submit button when chanign tabs.

* Decouple keyword time range validation from keyword time range preview in time range picker.

* Adding changelog.

* Fixing test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@maxiadlovskii maxiadlovskii maxiadlovskii approved these changes

@dennisoelkers dennisoelkers Awaiting requested review from dennisoelkers

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Search bar: Keyword time range time zone not set
2 participants
@linuspahl @maxiadlovskii