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

ui: Added support for now keyword in time range selector #4245

Closed
wants to merge 1 commit into from

Conversation

manojVivek
Copy link
Contributor

Resolves #4212

Copy link

alwaysmeticulous bot commented Jan 2, 2024

✅ Meticulous spotted zero visual differences across 396 screens tested: view results.

Last updated for commit fd28518. This comment will update as new commits are pushed.

Copy link
Contributor

@yomete yomete left a comment

Choose a reason for hiding this comment

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

lgtm!

@brancz
Copy link
Member

brancz commented Jan 3, 2024

I don't think the behavior requested in #4212 is quite what was implemented here.

Personally I'd be perfectly happy with just now as an "alias" for the current time.

I don't think the expected behavior is to literally replace the "now" with the timestamp, but rather keep "now" and transparently use the current timestamp when running the query. We would have to keep both the "meta value" like "now" in the URL as well as the "real" timestamp used to run the last query so we have all the state persisted in the URL.

I do think this also requires us to replace the built-in datetime picker with a custom one where one would be able to pick "now" (and potentially other meta values like "15min ago" in the future but I would skip other things for now).

@manojVivek
Copy link
Contributor Author

I don't think the expected behavior is to literally replace the "now" with the timestamp, but rather keep "now" and transparently use the current timestamp when running the query. We would have to keep both the "meta value" like "now" in the URL as well as the "real" timestamp used to run the last query so we have all the state persisted in the URL.

Yes, this what Yomi and I finalised on Discord as well, will change the implementation.

@manojVivek
Copy link
Contributor Author

Superseded by #4248

@manojVivek manojVivek closed this Jan 4, 2024
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.

Feature request: support now keyword in the time range
3 participants