Skip to content

chore: refactor ThematicDialog to functional component [DHIS2-18209]#3588

Open
BRaimbault wants to merge 69 commits intomasterfrom
chore/DHIS2-18209-ThematicDialog
Open

chore: refactor ThematicDialog to functional component [DHIS2-18209]#3588
BRaimbault wants to merge 69 commits intomasterfrom
chore/DHIS2-18209-ThematicDialog

Conversation

@BRaimbault
Copy link
Collaborator

@BRaimbault BRaimbault commented Nov 21, 2025

Parent: master
Child: #3584 > feat: support multiple split layers DHIS2-19542

Implements DHIS2-18209

Description

Refactor ThematicDialog to functional component.


Notes

Other potential improvements:

  • validate logic into a separate util
  • tabs into separate components
  • helper for repetitive dispatch wrappers
  • reduce the levels of nesting in the return function

My idea is to do it separately once all layer Dialog components are functional components.


Quality checklist

Add N/A to items that are not applicable.

  • Dashboard tested N/A
  • Cypress and/or Jest tests added/updated N/A
  • Docs added N/A
  • d2-ci dependencies replaced (analytics or maps-gl link https://github.com/dhis2/[lib]/pull/XXX) N/A
  • Tester approved (BR)

@dhis2-bot
Copy link
Contributor

dhis2-bot commented Nov 21, 2025

🚀 Deployed on https://pr-3588.maps.netlify.dhis2.org

@dhis2-bot dhis2-bot temporarily deployed to netlify November 21, 2025 09:31 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify November 25, 2025 15:18 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify November 25, 2025 15:50 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify November 25, 2025 16:05 Inactive
@BRaimbault BRaimbault marked this pull request as ready for review November 27, 2025 10:19
Copy link
Collaborator Author

@BRaimbault BRaimbault left a comment

Choose a reason for hiding this comment

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

Ready for review.

@dhis2-bot dhis2-bot temporarily deployed to netlify November 27, 2025 11:10 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify November 27, 2025 13:09 Inactive
edoardo
edoardo previously approved these changes Nov 28, 2025
Copy link
Member

@edoardo edoardo left a comment

Choose a reason for hiding this comment

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

It looks ok 👍

I would extract the tabs rendering code in separate components for readability and reduce the levels of nesting in the return function.

Copy link
Contributor

@HendrikThePendric HendrikThePendric left a comment

Choose a reason for hiding this comment

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

Sorry, I meant to also review this but got a bit sidetracked. What you have looks good, but obviously this component file is very large. Besides the suggestions @edoardo already made, you could consider extracting most of what you do before you return the JSX into one or several hook(s) so that you effectively end up with presentational component and a hook that encapsulates the business logic.

Without looking into the code too deeply, I'd say that perhapos here you could actually have one hook that encapsulates the "previous state", a second hook that is responsible for the validation (it can just return a validate function, and perhaps a third hook that runs all these effects and sets the errors.

Again, the idea above is without looking into things too deeply, perhaps you'd end up with too much interdependency between these 2-3 hooks and in that case you could just go with 1.

@dhis2-bot dhis2-bot temporarily deployed to netlify December 5, 2025 11:24 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify January 23, 2026 15:02 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify January 23, 2026 15:11 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify January 23, 2026 15:20 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify January 23, 2026 15:38 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify January 23, 2026 16:29 Inactive
@BRaimbault BRaimbault changed the base branch from master to chore/swiftshader-deprecation January 23, 2026 17:57
@dhis2-bot dhis2-bot temporarily deployed to netlify January 23, 2026 17:59 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify January 23, 2026 18:02 Inactive
Base automatically changed from chore/swiftshader-deprecation to master January 30, 2026 10:02
@dhis2-bot dhis2-bot temporarily deployed to netlify January 30, 2026 10:06 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify February 3, 2026 13:06 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify February 10, 2026 09:18 Inactive
@sonarqubecloud
Copy link

@dhis2-bot dhis2-bot temporarily deployed to netlify February 26, 2026 15:51 Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants