-
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
feat(detectors): Preserve page filters + env in DetectorLink #105055
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
feat(detectors): Preserve page filters + env in DetectorLink #105055
Conversation
| : detector.name; | ||
|
|
||
| // Preserve page filters when navigating to detector details | ||
| const query = pick(location.query, ['start', 'end', 'statsPeriod', 'environment']); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this necessary? The monitor details page reads from page filters, so it should be setting the date/env filters appropriately
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't seem to work right now? If I change the date range on the uptime monitors list and click a monitor, we lose the parameters
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, it looks like it works when selecting from the page filters but not if you use the chart
4ffb1b3 to
c2fcf22
Compare
|
This pull request has gone three weeks without activity. In another week, I will close it. But! If you comment or otherwise update it, I will reset the clock, and if you add the label "A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀 |
c2fcf22 to
07898ea
Compare
| href = link; | ||
| } else { | ||
| const pathname = link.pathname ?? ''; | ||
| const search = link.search ?? (link.query ? `?${qs.stringify(link.query)}` : ''); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Trailing ? in URLs when query object is empty
Low Severity
When openInNewTab is true and link.query is an empty object {}, the condition link.query ? evaluates to true since empty objects are truthy. However, qs.stringify({}) returns an empty string, resulting in search being just "?" with no parameters. This causes URLs to have a trailing ? when navigating without any page filters set. The check needs to verify the query object has properties, not just that it exists.
No description provided.