Conversation
| .ui-datepicker-calendar { | ||
| table-layout: fixed; | ||
|
|
||
| tbody td { |
There was a problem hiding this comment.
This is going deep into jQuery UI date picker itself.
Didn't really want to add classes there for this as well as other element selector below
| $this->markTestSkipped("Skipped this test on local dev environment."); | ||
| } | ||
| $maximumTotalFilesizesExpectedInMb = 63; | ||
| $maximumTotalFilesizesExpectedInMb = 64; |
There was a problem hiding this comment.
Bump this up by 1 mb as our filesize has gone over the previous maximum.
Also checked CoreHome.umd.js rose by only around 20Kb so not really that significant given we are creating more and more vue and ts files
| date: 'today', | ||
| period: 'day', | ||
| }), | ||
| createContextKey(createBaseContext({ category: 'General_Visitors', subcategory: 'General_Overview' })), |
There was a problem hiding this comment.
I love the createBaseContext() with overrides simplification 👍
| date: state.rollingDateParam || state.formattedAppliedAnchorDate, | ||
| period: state.appliedPeriod, | ||
| }; | ||
| } |
There was a problem hiding this comment.
I like the idea to deport the logic outside the vue components 👍
There was a problem hiding this comment.
It would be even better with a more explicit. The name applyFlow is perhaps not enough and the relation with the functions not direct.
The file could be PeriodSelector.applyButton.ts and the function:
- is ApplyButtonEnabled()
- getApplyButtonAction()
There was a problem hiding this comment.
changed the filename as well as the function name
| margin-right: 3px; | ||
| } | ||
|
|
||
| .compare-checkbox-text { |
There was a problem hiding this comment.
I like to use class selector instead of nested tags 👍
There was a problem hiding this comment.
Subjectif and non-blocking feedback
From the user point-of-view, I found the disable state not "transparent" enough.
After a fresh install:
- I open Matomo
- click on the date selector
- Yesterday is selected by default
- I cannot click on the calendar but it's not evident
Perhaps an opacity of 0.5 would be better?
| Current | opacity 0.5 |
|---|---|
 |
 |
plugins/CoreHome/vue/src/PeriodSelector/PeriodSelector.hashSync.spec.ts
Outdated
Show resolved
Hide resolved
chippison
force-pushed
the
calendar-clean-commit
branch
2 times, most recently
from
b404e5c to
e939b36
Compare
tzi
left a comment
tzi
left a comment
There was a problem hiding this comment.
I have an accessibility issue.
When the calendar is disabled, I can still focus each date with keyboard navigation.
Screen.Recording.2026-03-24.at.11.15.30.mov
|
It's better but, when I open the calendar, the focus doesn't go inside the calendar dropdown. Screen.Recording.2026-03-25.at.19.00.41.movPerhaps we should change the default focus on today / yesterday / etc. even if the calendar is enabled, because it's really what we will use 90% of the time. And it solves this problem |
chippison
force-pushed
the
calendar-clean-commit
branch
from
f9051aa to
58eb68e
Compare
@tzi |
|
I'm sorry for the nitpick. 😬
I have to use tab to see where I am first: Screen.Recording.2026-03-26.at.17.54.58.movCan we add the same outline on the presets? |
…ate for date range
…lighter than texts
…ly use keyboard navigation
…really needed tests, also making sure it does not use `require` and adding some helper functions to tests; updating screenshots as well
fixing issues found in code review updating less file
chippison
force-pushed
the
calendar-clean-commit
branch
from
58eb68e to
c3550f7
Compare
2 participants
Description
DEV-19886
This is for the second half of the calendar refresh.
Not a lot of functional changes from the #24139.
This is more of another round of component refactor, and some UI/CSS changes
This is just a copy of this PR.
I just cleaned up the commit history a bit.
Checklist
Review