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

Report argument modal translation #5483

Open
wants to merge 9 commits into
base: develop
Choose a base branch
from

Conversation

fergie-nz
Copy link
Contributor

Fixes #4951

πŸ‘©πŸ»β€πŸ’» What does this PR do?

Adds translations to report modal via json schema or UI schema.

Utilised inbuilt JSON form translation.

Maintains type safety of locale keys!

πŸ’Œ Any notes for the reviewer?

πŸ§ͺ Testing

You can test by running the 'Report-argument-modal-translation-example' branch which includes an example translation in the itemUsage report for both methods of adding translations

  • Go to reports
  • Open item usage report
  • See translation

Note - you may need to clear out existing reports in your database to view.
Delete reports, then run your back end. This will populate with updated report automatically

πŸ“ƒ Documentation

  • Part of an epic: But also documentation added in PR

@fergie-nz fergie-nz changed the base branch from develop to v2.4.0 November 19, 2024 23:36
@github-actions github-actions bot added this to the v2.4.0-RC2 milestone Nov 19, 2024
@github-actions github-actions bot added enhancement New feature or request Team Ruru πŸ¦‰ Andrei, Roxy, Chris, Ferg feature: reports labels Nov 19, 2024
@fergie-nz
Copy link
Contributor Author

Also made a PR to update reports repo docs:

https://github.com/msupply-foundation/open-msupply-reports/pull/118

@@ -154,8 +161,18 @@ const FormComponent = ({
// This allows "default" values to be set in the JSON schema
const handleDefaultsAjv = createAjv({ useDefaults: true });

const commonKeys = useMemo(() => new Set(Object.keys(common)), []);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

memoized keys in case checking Object.keys repeatedly is expensive. Unsure if it would be for how many keys we have.

@andreievg
Copy link
Collaborator

Looks great, does it work for our patient schema too:

Can you do an example of how it works for enums in that default patient schema ?

Also do you have a list of tasks to finish this issue fully ?

@fergie-nz
Copy link
Contributor Author

Looks great, does it work for our patient schema too:

Can you do an example of how it works for enums in that default patient schema ?

Also do you have a list of tasks to finish this issue fully ?

I hadn't considered the patient schema!

I'll have a look.

I do not have a list of tasks to finish this issue fully

@fergie-nz
Copy link
Contributor Author

Can you do an example of how it works for enums in that default patient schema ?

Yes with a small modification of how we add enums in our schema.

I will create an example of this and update the docs

@fergie-nz fergie-nz marked this pull request as draft November 20, 2024 00:09
Copy link

Bundle size difference

Comparing this PR to main

Old size New size Diff
4.97 MB 5.06 MB 92.06 KB (1.81%)

@fergie-nz
Copy link
Contributor Author

Can you do an example of how it works for enums in that default patient schema ?

Also do you have a list of tasks to finish this issue fully ?

I've had a look and this requires a few more changes.

It is a little over my head so will take a bit of time to figure out.
I've paused now because not obvious to me. Do you think worth looking into?

There are a couple of factors:

  • Our custom renderers don't accept the inbuilt JsonForms translation functionality
  • We stringify our patient schema in rust. If we are passing into rust, might as well translate there now rather than converting then re translating on the front end? If I am understanding how we manage the patient schema correctly

@roxy-dao roxy-dao modified the milestones: v2.4.0-RC2, V2.4.0-RC3 Nov 20, 2024
@fergie-nz fergie-nz marked this pull request as ready for review November 21, 2024 01:11
@roxy-dao roxy-dao modified the milestones: V2.4.0-RC3, V2.4.0-RC4 Nov 22, 2024
@roxy-dao roxy-dao modified the milestones: V2.4.0-RC4, V2.4.0-RC5 Nov 27, 2024
@roxy-dao roxy-dao removed this from the V2.4.0-RC5 milestone Nov 28, 2024
@roxy-dao roxy-dao added this to the V2.4.0-RC6 milestone Nov 28, 2024
@roxy-dao roxy-dao changed the base branch from v2.4.0 to develop November 28, 2024 22:44
@roxy-dao roxy-dao modified the milestones: V2.4.0-RC6, v2.4.1, v2.5.0 Nov 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request feature: reports Team Ruru πŸ¦‰ Andrei, Roxy, Chris, Ferg
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Reports: Translate arguments modal
3 participants