Add new useTimeQuery hook #75
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
🦋 Changeset detectedLatest commit: 57d0dc6 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
MikeShi42
previously approved these changes
Oct 29, 2023
|
oh actually one last thing, @joelseq can you add a changeset? then it should be good to go. |
MikeShi42
approved these changes
Oct 29, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Context
This PR relates to #53. From a chat with @MikeShi42 a while back we were discussing the UX of the time query presets (e.g. "Past 1h") and how it's kind of confusing atm. Once you click a preset, the label will remain as "Past 1h" and add a
fromandtoparam to the url. If after some time you refresh the page, the label will still say "Past 1h" but thefromandtoare the old values. At the same time, the existing useTimeQuery hook has a ton of complex logic which makes it very tricky to modify without causing regressions.This PR
Create a new version of the useTimeQuery hook (temporarily calling it useNewTimeQuery) and add detailed unit testing to it for all the different cases. This can eventually be used to replace all the existing callsites of the useTimeQuery hook. For the search page, we can move all the live tail functionality into a separate hook and integrate with this new time query hook to have better separation of concerns and make them both easier to maintain. I'll be making a separate PR for the live tail functionality.