fix(explore): cross events date selector allow 7d anytime within 30 days#116099
Conversation
github-actions
Bot
added
the
Scope: Frontend
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
📊 Type Coverage Diff✅ No new type safety issues introduced. Coverage: 93.58% |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 321ad50. Configure here.
nsdeschenes
left a comment
nsdeschenes
left a comment
There was a problem hiding this comment.
Couple of comments, nothing block worthy lmk what you think 😄
| const newDatetime = shouldUseMaxDateRange | ||
| ? { | ||
| period: `${maxPickableDays}d`, | ||
| period: `${maxDateRange}d`, | ||
| start: null, | ||
| end: null, | ||
| utc: datetime.utc, | ||
| } | ||
| : { | ||
| ...datetime, | ||
| period: | ||
| parsed.start || parsed.end || parsed.period || shouldUsePinnedDatetime | ||
| ? datetime.period | ||
| : null, | ||
| utc: parsed.utc || shouldUsePinnedDatetime ? datetime.utc : null, | ||
| }; | ||
| : shouldUseMaxPickableDays | ||
| ? { | ||
| period: `${maxPickableDays}d`, | ||
| start: null, | ||
| end: null, | ||
| utc: datetime.utc, | ||
| } | ||
| : { | ||
| ...datetime, | ||
| period: | ||
| parsed.start || parsed.end || parsed.period || shouldUsePinnedDatetime | ||
| ? datetime.period | ||
| : null, | ||
| utc: parsed.utc || shouldUsePinnedDatetime ? datetime.utc : null, | ||
| }; |
There was a problem hiding this comment.
nit: Instead of a nested ternary like this, could we use an if block, imo it's a tad easier to read
| ); | ||
| }); | ||
| }); | ||
|
|
There was a problem hiding this comment.
Should a test be added in checking that absolute date ranges don't get reset as well, just a small regression test 👀
| return new Date(start).getTime() < maxStart.getTime(); | ||
|
|
||
| if (maxDateRange) { | ||
| const maxTimeRange = maxDateRange * 24 * 60 * 60 * 1000; |
There was a problem hiding this comment.
Do we have a shared const somewhere in the FE for one day? Wondering if it's worth pulling that out just to be consistent and clearer what we're doing rather than having the raw numbers 🤔
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
nikkikapadia
deleted the
nikki/fix/explore-7d-cross-event-max-pickable-day
branch
2 participants
For cross event queries in explore we only limit the user to making 7 day queries at a time. The only issue was we needed to have the ability to query a 7 day range within the
maxPickableDays(of 90 days in this case) but there's no way to do that right now sincemaxPickableDaysdoesn't respectmaxDateRange. Ideally if there's amaxDateRangethe default time periods shown will be within that date range instead of just relying onmaxPickableDays. I've only made changes to the logic if BOTHmaxPickableDaysandmaxDateRangeare passed in.I took a look to see if there are any other parts of the app that use
maxDateRangeand i don't see any references to it so this shouldn't cause odd behaviour on any other date selector :)Here's what it's looking like:
Screen.Recording.2026-05-22.at.11.41.22.AM.mov