feat: use PeriodDimension in ThematicDialog #3407
Conversation
|
🚀 Deployed on https://pr-3407.maps.netlify.dhis2.org |
| @@ -10,6 +10,10 @@ | |||
| height: 36px; | |||
| } | |||
|
|
|||
| .radioGroup.compact fieldset>div>div>label { | |||
There was a problem hiding this comment.
Is it possible to use the same spacing as is on line 25?
| /> | ||
| )} | ||
| {periodError && ( | ||
| <div key="error" className={styles.error}> |
There was a problem hiding this comment.
What is the purpose of the key attribute?
| display: flex; | ||
| align-items: center; | ||
| line-height: 1; | ||
| gap: 8px; |
|
I noticed a few things when entering the start or end date. Maybe it's working as expected, or maybe it should be improved, not sure:
|
|
The line heights feel a little tight here, especially since there is room to make a little more space: |
|
The info says that at least 2 periods must be added in order to enable timeline or split view. But if I add just 1 period, both Ok, so I see that if I had chosen only "This month", then those options would still be disabled. But if I choose only "Last 6 months", then they are enabled. I guess "this month" counts as a single period, while "last 6 months" counts as 6 periods? If that's the case, then I think maybe the info text needs to be a little clearer. |
|
Just to confirm, with this implementation, the "bug" mentioned in this Jira issue will go away. Is it confirmed that we want the start/end dates to not be cleared even if they aren't in use? |
| return this | ||
| } | ||
|
|
||
| /* eslint-disable max-params */ |
There was a problem hiding this comment.
Maybe send an object instead of disabling max-params
| .validateStage('Inpatient morbidity and mortality') | ||
| .selectTab('Period') | ||
| .selectPeriodType('Start/end dates') | ||
| .typeStartDate(`${CURRENT_YEAR - 5}-02-01`) |
There was a problem hiding this comment.
With the use of the "-5" in several places I wonder if it might be informative to future us to create a test object, for example:
{
programName: 'Inpatient morbitity and mortality',
yearWithData: CURRENT_YEAR - 5
}
Maybe other properties as well?
| excludedPeriodTypes={ | ||
| systemSettings.hiddenPeriods | ||
| } | ||
| height={'250px'} |
There was a problem hiding this comment.
I don't think you need the curly braces.
| @@ -631,15 +701,14 @@ class ThematicDialog extends Component { | |||
| ) | |||
| } | |||
|
|
|||
| if (!period && periodType !== START_END_DATES) { | |||
| if (periods?.length === 0 && periodType !== START_END_DATES) { | |||
There was a problem hiding this comment.
If periods is undefined, then this first condition will not be true. Is that expected behavior?
| @@ -23,8 +24,15 @@ const ManageLayerSourcesModal = ({ onClose }) => { | |||
| const { managedLayerSources, showLayerSource, hideLayerSource } = | |||
| useManagedLayerSourcesStore() | |||
|
|
|||
| useKeyDown('Escape', onClose) | |||
There was a problem hiding this comment.
I'm thinking that maybe we should pull changes that aren't related to the period dimension into a separate PR.
Implement: DHIS2-15796
Linked @dhis2/analytics PR: dhis2/analytics#1735
Overview
Main changes are:
PeriodDimensionperiod selector (transfer-style) for preset periods (in thematic dialog)StartEndDatefor start-end dates periods (in thematic, events and tracked entity dialogs)RenderingStrategythat controls the use of single map / timeline / split-map (in thematic dialog)SegmentedControlto switch between preset periods and start-end dates (in thematic dialog)Timelinecomponent to support multiple periods and from class to functional component (for thematic layers)Also improved keyboard navigation is introduced. The original object was to only to enable typing dates properly. In the end a number of files are modified to introduce consistency. Reviewing them should be quick so it might be a good idea to start here. A few other quality of life improvements included as well. All these changes are listed at the end.
1. Multiple periods selection
2.
PeriodDimensionThe new period selector used is
PeriodDimension(transfer-style) is defined in@dhis2/analytics.3.
StartEndDateWe updated and rename
StartEndDatestoStartEndDateto align with the Linelisting app but with an updated date pickerCalendarInputfrom@dhis2/uiwhich supports multiple calendars.EventDialogandTrackedEntityDialogare also updated to useStartEndDate.4.
RenderingStrategyWe updated the logic to account for multiple periods selection.
5.
SegmentedControl6.
TimelineSmaller quality of life improvements are included:
Keyboard navigation
Added "x" close modal buttons:
Small bug fixes
Missing:
Checklist: