Skip to content

Improve preset period selector behavior#24584

Open
tzi wants to merge 8 commits into
5.x-devfrom
dev-20172
Open

Improve preset period selector behavior#24584
tzi wants to merge 8 commits into
5.x-devfrom
dev-20172

Conversation

@tzi

@tzi tzi commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

Description

This PR improves the preset period selector behavior:

  • Selecting a preset do not disabled the calendar anymore: It looks more like a "Period type + date" named shortcut
  • It also behaves like an indicator: when the users select a preset manually by navigating via period/date combinations it is marked as selected as well.
Screen.Recording.2026-06-01.at.23.58.30.mov

issue dev-20172 #24425

Test Scenario

  1. Click any preset:
  • it should select a period type and a calendar date
  • Ex: today select "Day period" + "day date"
  • you could click "Apply" to apply
  1. Double Click any preset:
  • it should select a period type and a calendar date
  • It's applied without having to click on "Apply"
  1. Manual selection
  • it should select a preset if it matches the manual selection
  • Ex: I select "This week" and then select the week before thanks to the calendar, if I reopen the PeriodSelector we can see that "Last week" is selected

Checklist

  • [✔] I have understood, reviewed, and tested all AI outputs before use
  • [✔] All AI instructions respect security, IP, and privacy rules

Review

@tzi tzi added this to the 5.11.0 milestone Jun 1, 2026
@tzi tzi marked this pull request as ready for review June 2, 2026 00:57
@tzi tzi requested a review from a team June 2, 2026 00:57

@chippison chippison left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We also seem to have removed 'rolling dates' (today, last7, last30, thisMonth etc) in the url.

This will mean if a period is saved/bookmarked, when the that bookmarked date is returned to, it will no longer have the same context.

Meaning if I chose to get a report for the last month (lets say this month is: June, then last month is May) and bookmarked it,

Previously, once we go to the bookmarked page after a month, it will show 'last months' report (show June report since its July after a month),
Now, it will still show May's report

This seems to be a previous needed behaviour since we put in a bunch of code to make this work.

If we decide not to honor the rolling dates anymore, there would be a bunch of dead code that needs cleaning up

Comment thread plugins/CoreHome/vue/src/PeriodSelector/PeriodSelector.vue Outdated
Comment thread plugins/CoreHome/vue/src/PeriodSelector/PeriodSelector.vue Outdated
Comment thread plugins/CoreHome/vue/src/PeriodSelector/PeriodSelector.vue Outdated
Comment thread plugins/CoreHome/vue/src/PeriodSelector/PeriodSelector.vue
@sgiehl sgiehl modified the milestones: 5.11.0, 5.12.0 Jun 5, 2026

@chippison chippison left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There are some double/redundant functions that we can remove.

Also a 'dead' css rule as well

Comment thread plugins/CoreHome/vue/src/PeriodSelector/PeriodSelector.vue Outdated
Comment thread plugins/CoreHome/vue/src/PeriodSelector/PeriodSelector.vue
Comment thread plugins/CoreHome/vue/src/PeriodSelector/PeriodSelector.vue Outdated
Comment thread plugins/CoreHome/vue/src/PeriodSelector/PeriodSelector.vue Outdated
Comment on lines +326 to +329
const exactMatch = PRESET_DATE_RANGES.find((preset) => {
const resolvedPreset = resolvePresetDateRange(preset.id, todayInput);
return resolvedPreset.period === period && resolvedPreset.date === date;
});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nitpicking here:
You seem to have called PRESET_DATE_RANGES.find twice in this function. (line: 326 and 337)
Could be done in one

@chippison chippison left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There is a regression when clicking through the dates:

  1. Rolling dates disappear/go back to real dates when reclicking on the preset for last7, last30, last90
rolling-dates-disappear.mov
  1. When clicking on a preset date, I can no longer add comparisons to it unless I change preset first
cannot-compare-on-same-date.mov

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants