Skip to content
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

fix: indent org unit with no child and refresh lists on updates #473

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

flaminic
Copy link
Contributor

No description provided.

Copy link

netlify bot commented Dec 18, 2024

Deploy Preview for dhis2-maintenance-app-beta ready!

Name Link
🔨 Latest commit e3e0e51
🔍 Latest deploy log https://app.netlify.com/sites/dhis2-maintenance-app-beta/deploys/6762c2a312af4f0008f0ad5e
😎 Deploy Preview https://deploy-preview-473.maintenance-app-beta.netlify.dhis2.org
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@flaminic flaminic force-pushed the small-org-unit-fixes branch from c2dd1e1 to 3d68912 Compare December 18, 2024 09:52
@flaminic flaminic requested a review from Birkbjo December 18, 2024 09:54
Copy link
Member

@Birkbjo Birkbjo left a comment

Choose a reason for hiding this comment

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

Did we decide to remove the "expand"-all button as well?

@@ -53,6 +55,9 @@ export const useOnSubmitEdit = <TFormValues extends IdentifiableObject>({
message: i18n.t('Saved successfully'),
success: true,
})
queryClient.invalidateQueries({
Copy link
Member

@Birkbjo Birkbjo Dec 18, 2024

Choose a reason for hiding this comment

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

I think this makes a lot of sense, however I'm not sure if we want to couple the sectionPath to the resource?
getSectionPath is for the router, and if we in the future would want to change those paths.

I think it's more correct to use section.namePlural if we want to do this.

Initially I was thinking to add refetchOnMount: true on the queries for the list. That should fix the problem in this case. But I do agree that invalidating the query makes a lot of sense as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good, i'll chande it to namePlural and keep it as it is then (i do like being explicit about it)

@@ -87,7 +87,9 @@ export const OrganisationUnitRow = ({
onClick={row.getToggleExpandedHandler()}
></Button>
</>
) : null}
) : (
Copy link
Member

Choose a reason for hiding this comment

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

Did we not decide to remove the expand all as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes i can do that as well

@flaminic flaminic requested a review from Birkbjo December 18, 2024 12:40
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.

2 participants