Skip to content

Feat/new data entry app compatibility fixes#174

Merged
MiquelAdell merged 14 commits intodevelopmentfrom
feat/new-data-entry-app-compatibility-fixes
Apr 24, 2026
Merged

Feat/new data entry app compatibility fixes#174
MiquelAdell merged 14 commits intodevelopmentfrom
feat/new-data-entry-app-compatibility-fixes

Conversation

@deeonwuli
Copy link
Copy Markdown
Contributor

@deeonwuli deeonwuli commented Apr 1, 2026

📌 References

📝 Implementation

  • Fix period selection in new Data Entry app: In useDataEntrySelector, the period is now read from dhis2.de.currentPeriodId first (falling back to dhis2.de.getSelectedPeriod()?.iso). This adds compatibility with the new DHIS2 Data Entry app which exposes currentPeriodId instead of relying solely on getSelectedPeriod().
  • Remove "You need to enable JavaScript" message: Added removeJSDisabledMessage() in index.tsx that strips the -style text node from the host app's DOM when the React app initializes inside the DHIS2 Data Entry app.
  • CSS/styling fixes: To ensure consistency between the old and new data entry apps
    • Tab navigation font size matches the legacy app: StyledTab, the navigation buttons, and the position indicator were sized in rem/em. Relative units resolve against the host app's root font-size, which differs between the new and legacy data entry apps, so tabs rendered at different pixel sizes in each. Fixed by pinning them to explicit px values so the form renders identically regardless of the host's root styles.
    • Sections no longer overflow the iframe: With the previous default auto table layout, DHIS2's DataTable sized columns from their content and spilled past the iframe edge. Switched GridForm, GridWithCatOptionCombos and TableForm to layout="fixed" with width="100%", pinned the period column to 120px, defaulted the first column to auto, and made column-header content fill its cell so text wraps instead of overflowing.
    • Alert rules escape the iframe and keep their styles: Alert toasts used position: fixed, which anchors to the iframe viewport rather than the browser viewport, so they were clipped at the iframe edge in the new app. Added an IframeEscapePortal that portals children into the topmost same-origin document and re-points `styled-components' StyleSheetManager and MUI v4's JSS provider at that document. Without the re-point, the portalled nodes would render unstyled
    • Column headers composite correctly over any host background: section.styles.columns.backgroundColor can be an rgba() value, which blends with whatever sits behind the th. That backdrop is white in the legacy app but a different color inside the new app's iframe, so the same config produced visibly different header colors. Painted the configured color as a linear-gradient over a solid white background-color on the th itself, so the result no longer depends on the host background. Solid colors are unaffected.

🎨 Screenshots

Screen.Recording.2026-04-07.at.11.03.31.mov

🔥 Notes to the tester

#869cn5qjz

@deeonwuli deeonwuli changed the base branch from master to development April 1, 2026 22:28
@bundlemon
Copy link
Copy Markdown

bundlemon Bot commented Apr 1, 2026

BundleMon

No change in files bundle size

Groups updated (1)
Status Path Size Limits
Build Folder
./**/*
1.22MB (+903B +0.07%) +20%

Final result: ✅

View report in BundleMon website ➡️


Current branch size history | Target branch size history

@deeonwuli deeonwuli marked this pull request as ready for review April 7, 2026 10:09
Copy link
Copy Markdown
Contributor

@MatiasArriola MatiasArriola left a comment

Choose a reason for hiding this comment

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

Thanks @deeonwuli! Nice to see the integration with the new app. Please check my comments inline.

currentOrganisationUnitId: Id;
currentDataSetId: Id;
getSelectedPeriod(): Maybe<Period>;
currentPeriodId: Id;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It is worth declaring this currentPeriodId prop as optional to better reflect the types in reality. Also consider adding a simple comment stating the prop is v42+ only, and getSelectedPeriod is for < 42.

Comment thread src/index.tsx
Comment on lines +54 to +67
function removeJSDisabledMessage() {
const root = document.getElementById("root");
if (root?.parentNode) {
Array.from(root.parentNode.childNodes).forEach(node => {
if (
node.nodeType === Node.TEXT_NODE &&
node.textContent?.includes(i18n.t("You need to enable JavaScript"))
) {
node.remove();
}
});
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Interesting! I assume the new data entry app started rendering the noscript tag from index.html.

I would consider some alternative for this implementation. It is fine as a workaround, but let's check if there is a better solution. This adds an extra unnecessary translation key, because at this point i18n is not initialized.

If this is the <noscript> tag from index.html, we could:

  • try adding an id to the noscript element and remove it by its id instead of scanning nodes document.getElementById("js-disabled-message")?.remove()
  • or keep current approach, just define the seach text as a constant, remove the i18n call, and remove it from translation files

I would remove the noscript tag entirely, but since this app has other variants like the configurator we might want to keep it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch on the i18n call — you're right, it's not initialized at that point.

I went with the second approach: the search text is now a plain string constant ("You need to enable JavaScript"), the i18n.t() call has been removed, and the translation key has been removed from all .po/.pot files.

I also tried the getElementById approach first, but it doesn't work here — the text node comes from the host Data Entry app's HTML (DHIS2 v42+), not from our index.html. So our is never present in the deployed context. We match by text content since the host element has no id.

@deeonwuli deeonwuli requested a review from MatiasArriola April 8, 2026 17:39
Copy link
Copy Markdown
Contributor

@MatiasArriola MatiasArriola left a comment

Choose a reason for hiding this comment

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

Nice, thanks @deeonwuli!!

@MiquelAdell
Copy link
Copy Markdown
Contributor

Hi @deeonwuli I checked the new form by using the TUB Annual data as a test and found some things to fix:

  1. the tab navigation has a font too small
  2. previous next has a font too small
  3. sections break the container width
image
  1. I can not get validation rules to trigger nor with our "inline" system (when we change a value) nor with the "Run Validation" button:
image

@deeonwuli deeonwuli requested a review from MatiasArriola April 23, 2026 11:00
@MiquelAdell MiquelAdell merged commit 77b4ff6 into development Apr 24, 2026
6 checks passed
@MiquelAdell MiquelAdell mentioned this pull request Apr 24, 2026
1 task
@MatiasArriola
Copy link
Copy Markdown
Contributor

Hello, I'm leaving some comments regarding the second part of this PR:

  • IframeEscapePortal: I think it is a nice workaround idea to have this React portal and the muiProvidersCache, but we could improve this implementation:
    • useMemo contains side-effects. It should be used for computing and caching expensive values, not for side-effects. Instead use useEffect.
    • If refactoring into useEffect, we could also have a cleanup function for the div and style dynamically inserted (if appropriate), like it is being done in useParentDataEntryAppStyles.ts
  • useParentDataEntryAppStyles duplicates the IframeEscapePortal logic to get the parent window with handling for Cross-Origin ancestor. Maybe we can refactor for code reuse.

@MiquelAdell
Copy link
Copy Markdown
Contributor

MiquelAdell commented Apr 28, 2026

@deeonwuli you can go ahead and apply these changes @MatiasArriola suggested whenever you have a moment. You can reopen the PR if you want or create a new one, whatever is easiest for you. If you create a new one please link this one there.

Thanks!

EDIT: I see it is already here:
#183 (comment)

@MiquelAdell MiquelAdell deleted the feat/new-data-entry-app-compatibility-fixes branch April 28, 2026 07:54
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.

3 participants