Feature/download template redesign by gqcorneby · Pull Request #342 · EyeSeeTea/Bulk-Load

gqcorneby · 2025-01-22T08:38:26Z

📌 References

Issue: Closes #8694wfy5f https://app.clickup.com/t/8694wfy5f

📝 Implementation

added a section component to reuse styles and collapse function for the form sections
divided fields into sections
rearranged advanced options
updated toggle filterOrgUnits logic. Remove clearing of populate, populate dates, and selectedOrgUnits (more notes in reviewers section)
hide populate section until an orgUnit is selected. Previously depended on the org unit checkBox

🔥 Notes for the reviewer

hide populate section until an orgUnit is selected, otherwise the section is empty
updated behavior of selecting organization unit.
- PREVIOUSLY: when toggled, the following were cleared: selectedOrgUnits, populate, populate dates
- NOW: collapsing implies a visual change not data change, so I removed data updates
I opted to use up down arrows instead of left right as indicated in the task document. Seems to fit more with how the section collapses.
I used the "Organisation Unit" label beside the toggle with an indicator of how many is selected (found it useful while testing so included it for now)

📹 Screenshots/Screen capture

📑 Others

- add sections in download template page - collapsible orgunit section instead of a checkbox - rearrange advanced options

ifoche · 2025-01-22T08:38:30Z

Task linked: CU-8694wfy5f Redesign download template page

tokland

[code-only review] Looking good! just some minor details:

tokland · 2025-01-22T11:15:28Z

src/webapp/components/collapsible-section/Section.tsx

+    iconPos = "right",
+    classProps = {},
+    arrowStyle = "upDown",
+}: SectionProps) => {


That's not wrong at all, but typically we prefer to write props: SectionProps here and unpack in the first line of the component, so the signature stays single-line and more readable (subjective!).

tokland · 2025-01-22T11:17:47Z

src/webapp/components/collapsible-section/Section.tsx

+    arrowStyle?: arrowStyle;
+}
+
+export const Section = ({


I see you preferred to define children explicitly in the props instead of using React.FC. We typically use FC, BUT since it adds always children (that's why some people don't like FC), we lose some control, so let's go with this. Eventually, we can decide on creating our own "FCWithProps" or whatever, but no need to to it here.

tokland · 2025-01-22T11:21:52Z

src/webapp/components/collapsible-section/Section.tsx

+    return (
+        <Paper elevation={elevation} className={`${classes.paper} ${classProps.section}`}>
+            <div
+                className={`${classes.header} ${leftIcon} ${classProps.header} ${isOpen ? "" : classes.noBorder}`}


If I am not mistaken, when leftIcon is null, it will appear "null" in the class list. There are packages for that (classnames), but you can also do this: _.compact([name1, name2, name3]).join(" ")

tokland · 2025-01-22T11:28:22Z

src/webapp/components/collapsible-section/Section.tsx

+                {title}
+                {collapsible && !leftIcon && <CollapsibleToggle isOpen={isOpen} arrowStyle={arrowStyle} />}
+            </div>
+            <div className={`${classProps.content}`} style={{ display: isOpen ? "" : "none" }}>


Whenever possible, we strive for immutable props (memoized components will benefit from that).

tokland · 2025-01-22T11:29:09Z

src/webapp/components/collapsible-section/Section.tsx

+    arrowStyle: "upDown" | "rightLeft";
+}
+
+export const CollapsibleToggle = ({ isOpen, arrowStyle }: CollapsibleToggleProps) => {


It's not used externally, right? we can remove the export.

tokland · 2025-01-22T11:35:45Z

src/webapp/components/template-selector/TemplateSelector.tsx

            showLanguage: false,
        }
    );
+    const [showAdvancedOptions, setShowAdvancedOptions] = useState<boolean>(true);


Not wrong, but we don't typically pass a type if TS can infer it.

tokland · 2025-01-22T11:38:19Z

src/webapp/components/template-selector/TemplateSelector.tsx

+        // const isCustomProgram = state.templateType === "custom" && state.type !== "dataSets";
+        // const populate = isCustomProgram && filterOrgUnits;
+        // setState(state => ({ ...state, populate }));
+        // clearPopulateDates();


Typically, commented code should be simply removed, we can use git to go back (We may do this for some temporal/testing code, but not in general)

tokland · 2025-01-22T11:42:04Z

src/webapp/components/template-selector/TemplateSelector.tsx

-                {models.length > 1 && (
+            <Section
+                title={<h3 className={classes.title}>{i18n.t("Template")}</h3>}
+                classProps={{ section: classes.section }}


[immutable props] I think we can simply pass "classes" here (unless we really don't want to pass the other keys that Section uses (header?). If that's the case, we can use useMemo to build the object. We would be using useMemo not because it's a costly operation, but to keep the value/prop immutable.

tokland · 2025-01-22T11:47:24Z

src/webapp/components/template-selector/TemplateSelector.tsx

+                        label={i18n.t("Split data entry tabs by section")}
+                    />
+                )}
+


[not to change] The return JSX expression of this component is huuuge (608-294 lines). That's not your fault, you are just moving already existing parts. So this is a note for the future: whenever a component grows too much, let's abstract conceptual units and create new components. But again, it does not make sense to do it now (it's more fitting for a general refactoring/polishing task)

tokland · 2025-01-22T11:51:19Z

src/webapp/components/collapsible-section/Section.tsx

+    elevation = 1,
+    iconPos = "right",
+    classProps = {},
+    arrowStyle = "upDown",


About the expand/collapse styles, I checked what DHIS2 does, and... it depends:

orgUnits tree (icon: left the name): right/down
maps (icon: top-right): up/down

https://play.im.dhis2.org/stable-2-40-6/dhis-web-maintenance/index.html#/list/organisationUnitSection/organisationUnit

https://play.im.dhis2.org/stable-2-40-6/dhis-web-maps/#/GlCLRPPLsWF

I have no strong opinion on this, so PM to decide.

- update section classProps to more specific classNames - update arrowStyle. Add down_right

…om/EyeSeeTea/Bulk-Load into feature/download-template-redesign

tokland

Just some minor details and we're good to go.

Run yarn localize before a push, to check if there are changes on the i18n files (and the commit then, ofc). There are some changes, I think.

src/webapp/components/collapsible-section/Section.tsx

src/webapp/components/template-selector/TemplateSelector.tsx

tokland

All good! Just added a commit for some useMemo and a comment about i18.

tokland · 2025-01-27T12:51:23Z

i18n/es.po

-msgstr "<Sin valor>"
+#, fuzzy
+msgid "Populate"
+msgstr "Plantilla"


We typically check fuzzy tags in PO files (gettext adds them, meaning: "I think this is the translation for a new literal I found, which is very similar to another existing, but I am not sure"). We review if correct and the remove the line "#, fuzzy".

BUT, as all the po files already contain fuzzy tags (a lot), you may leave this task to another "cleaning" PR, not in this feature. Confirm with your PM.

tokland · 2025-01-27T12:56:10Z

src/webapp/components/template-selector/TemplateSelector.tsx

    const selected = state.id && state.templateId ? getOptionValue({ id: state.id, templateId: state.templateId }) : "";

-    const subSectionClasses = useMemo(() => ({ sectionHeader: classes.subSectionHeader, sectionPaper: classes.subSection }), [classes]);
+    const subSectionClasses = { sectionHeader: classes.subSectionHeader, sectionPaper: classes.subSection };


[ALREADY COMMITED] But here we do need useMemo to keep it immutable, as it's an object directly passed as a prop (string, numbers, booleans, null, undefined are directly comparable, array/objects/pretty much everything else, not)

tokland

I got the re-review but I think there is nothing new from my previous approval. Approving again. Let me know if there's something specific I should look at.

# Conflicts: # i18n/en.pot # i18n/es.po # i18n/fr.po # i18n/pt.po # i18n/ru.po # src/webapp/components/template-selector/TemplateSelector.tsx

bundlemon · 2025-09-17T10:17:30Z

BundleMon

No change in files bundle size

Groups updated (1)

Status	Path	Size	Limits
✅	Build Folder _./*/	1.85MB (+1.59KB +0.08%)	+20%

Final result: ✅

View report in BundleMon website ➡️

Current branch size history | Target branch size history

gqcorneby added 2 commits January 22, 2025 15:35

download template page redesign

4c2d642

- add sections in download template page - collapsible orgunit section instead of a checkbox - rearrange advanced options

add periods section

3f125d4

gqcorneby requested review from adrianq and tokland January 22, 2025 08:38

tokland reviewed Jan 22, 2025

View reviewed changes

gqcorneby added 3 commits January 24, 2025 15:16

add periods section

3956019

cleanup section and templateselector

bfbee3f

- update section classProps to more specific classNames - update arrowStyle. Add down_right

Merge branch 'feature/download-template-redesign' of https://github.c…

6c03661

…om/EyeSeeTea/Bulk-Load into feature/download-template-redesign

gqcorneby requested a review from tokland January 24, 2025 07:29

tokland reviewed Jan 24, 2025

View reviewed changes

add buildClassNames, clean up, prettify

7d1a064

gqcorneby requested review from tokland January 27, 2025 03:55

gqcorneby and others added 2 commits January 27, 2025 12:29

cleanup help icon

b8b34e7

Use useMemo to keep subSectionClasses immutable

8704405

tokland approved these changes Jan 27, 2025

View reviewed changes

tokland approved these changes Feb 5, 2025

View reviewed changes

Merge branch 'development' into feature/download-template-redesign

85115eb

MiquelAdell self-requested a review September 17, 2025 08:39

gqcorneby added 4 commits September 17, 2025 17:36

Merge branch 'development' into feature/download-template-redesign

eeadde7

# Conflicts: # i18n/en.pot # i18n/es.po # i18n/fr.po # i18n/pt.po # i18n/ru.po # src/webapp/components/template-selector/TemplateSelector.tsx

update help icon. Use outlined

06e6a8c

translations

01ba970

fix test errors

b32483a

gqcorneby and others added 4 commits September 18, 2025 13:57

add label help

405033e

translations

7729389

update default collapsed sections

1a8be64

change default OU filter from CAPTURE to SELECTED

df419b3

MiquelAdell approved these changes Sep 24, 2025

View reviewed changes

MiquelAdell merged commit afc5569 into development Sep 24, 2025
6 checks passed

MiquelAdell deleted the feature/download-template-redesign branch September 24, 2025 11:30

Conversation

gqcorneby commented Jan 22, 2025 • edited by tokland Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📌 References

📝 Implementation

🔥 Notes for the reviewer

📹 Screenshots/Screen capture

📑 Others

Uh oh!

ifoche commented Jan 22, 2025

Uh oh!

tokland left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tokland Jan 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tokland Jan 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tokland Jan 22, 2025

Choose a reason for hiding this comment

Uh oh!

tokland Jan 22, 2025

Choose a reason for hiding this comment

Uh oh!

tokland Jan 22, 2025

Choose a reason for hiding this comment

Uh oh!

tokland Jan 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tokland Jan 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tokland Jan 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tokland Jan 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tokland Jan 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tokland left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tokland left a comment

Choose a reason for hiding this comment

Uh oh!

tokland Jan 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tokland Jan 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tokland left a comment

Choose a reason for hiding this comment

Uh oh!

bundlemon bot commented Sep 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

BundleMon

Uh oh!

Uh oh!

gqcorneby commented Jan 22, 2025 •

edited by tokland

Loading

tokland left a comment •

edited

Loading

tokland Jan 22, 2025 •

edited

Loading

tokland Jan 22, 2025 •

edited

Loading

tokland Jan 22, 2025 •

edited

Loading

tokland Jan 22, 2025 •

edited

Loading

tokland Jan 22, 2025 •

edited

Loading

tokland Jan 22, 2025 •

edited

Loading

tokland Jan 22, 2025 •

edited

Loading

tokland left a comment •

edited

Loading

tokland Jan 27, 2025 •

edited

Loading

tokland Jan 27, 2025 •

edited

Loading

bundlemon bot commented Sep 17, 2025 •

edited

Loading