-
Notifications
You must be signed in to change notification settings - Fork 162
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: Month granularity for date-range-picker #3077
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3077 +/- ##
==========================================
+ Coverage 96.44% 96.45% +0.01%
==========================================
Files 790 791 +1
Lines 22246 22461 +215
Branches 7229 7345 +116
==========================================
+ Hits 21455 21665 +210
- Misses 784 789 +5
Partials 7 7 ☔ View full report in Codecov by Sentry. |
7491fa5
to
7f28657
Compare
e8bd8f5
to
f61d398
Compare
f61d398
to
74458ef
Compare
74458ef
to
73641df
Compare
d63e7be
to
24aba24
Compare
24aba24
to
0773794
Compare
dacbb1c
to
ae77f9e
Compare
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.
Approved to merge, but still looking forward for a refactoring around that day/month strategy switch
); | ||
} | ||
|
||
const isInFirstGrouping = (granularity: DateRangePickerProps['granularity'], date: Date) => { |
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.
okay, got it
this.isMonthPicker = isMonthPicker; | ||
} | ||
|
||
getStrategy(): DatePickerStrategyUtils { |
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.
This is not how pattern should work. There is still the same number of isMonthPicker
checks, just in a different place.
It could be something like this
interface GridUtils {
getItemKey(rowIndex: number, columnIndex: number): string;
isSameItem: (date1: Date, date2: Date): boolean;
isSamePage: (date1: Date, date2: Date): boolean;
... etc
}
const monthUtils: GridUtils = {
// note, argument types are not needed, they are coming with `GridUtils` type declaration
getItemKey: (rowIndex, columnIndex) => `Month ${rowIndex * 3 + rowItemIndex + 1}`,
isSameItem: isSameMonth,
isSamePage: isSameYear,
... etc
}
const dayUtils: GridUtils = {
getItemKey: (rowIndex, columnIndex) => `${rowIndex}:${rowItemIndex}`,
isSameItem: isSameDay,
isSamePage: isSameMonth,
... etc
}
// pull the right strategy depending on the mode
const gridUtils = isMonthPicker ? monthUtils : dayUtils
"strategy" == "utils" in this code. The name does not really matter, the concept does. Because it makes code shorter and easier to follow
Month granularity for date-range-picker
The Date Picker component has a granularity prop that can be set to "month" which disallows the selection of day and allows the selection of month. This functionality is now being extended onto Date Range Picker component, so it will also support month range selection
issues: [AWSUI-18630, AWSUI-20124, AWSUI-8139]
Previous PRs for this project
How has this been tested?
test coverage expanded to include both unit tests and integration tests for the DateRangePicker component utilizing the 'month' granularity setting. These tests now encompass rendered iterations of the components, ensuring comprehensive validation of this specific functionality.
Review checklist
The following items are to be evaluated by the author(s) and the reviewer(s).
Correctness
CONTRIBUTING.md
.CONTRIBUTING.md
.Security
checkSafeUrl
function.Testing
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.