(BREAKING) O3-3037 : Refactor components and rename files #211

pirupius · 2024-04-24T08:33:26Z

Requirements

This PR has a title that briefly describes the work done including the ticket number. If there is a ticket, make sure your PR title includes a conventional commit label. See existing PR titles for inspiration.
My work conforms to the OpenMRS 3.0 Styleguide and design documentation.
My work includes tests or is validated by existing tests.

Summary

This PR introduces BREAKING changes with multiple refactors to file names and components

Removes ohri scoped naming
Renames typings

Screenshots

Related Issue

https://openmrs.atlassian.net/browse/O3-3037

Other

pirupius · 2024-04-24T12:43:38Z

src/registry/inbuilt-components/inbuiltControls.ts

  {
-    name: 'OHRIText',
-    component: OHRIText,
+    name: 'TextField',


We'll be deprecating this value as documented in O3-3109

github-actions · 2024-04-24T14:41:22Z

Size Change: -238 kB (-16%) 👏

Total Size: 1.22 MB

Filename	Size	Change
`dist/784.js`	0 B	-237 kB (removed)	🏆

ℹ️ View Unchanged

Filename	Size	Change
`dist/100.js`	2.41 kB	0 B
`dist/154.js`	80.8 kB	+1 B (0%)
`dist/17.js`	16.4 kB	0 B
`dist/225.js`	2.57 kB	+1 B (0%)
`dist/248.js`	108 kB	0 B
`dist/29.js`	220 kB	0 B
`dist/311.js`	237 kB	0 B
`dist/327.js`	1.58 kB	0 B
`dist/353.js`	3.02 kB	0 B
`dist/459.js`	6.07 kB	-2 B (0%)
`dist/505.js`	7.46 kB	-1 B (0%)
`dist/540.js`	2.63 kB	0 B
`dist/561.js`	487 B	0 B
`dist/599.js`	2.51 kB	+1 B (0%)
`dist/606.js`	2.23 kB	+1 B (0%)
`dist/633.js`	95 kB	+1 B (0%)
`dist/791.js`	9.8 kB	0 B
`dist/828.js`	94 kB	0 B
`dist/859.js`	0 B	-879 B (removed)	🏆
`dist/886.js`	6.94 kB	0 B
`dist/942.js`	487 B	0 B
`dist/993.js`	3.08 kB	0 B
`dist/main.js`	312 kB	+121 B (0%)
`dist/openmrs-form-engine-lib.js`	3.77 kB	+2 B (0%)

_{compressed-size-action}

rebase cleanups reference refactors

denniskigen

LGTM. Thanks, @pirupius.

denniskigen · 2024-04-24T16:49:34Z

src/components/inputs/content-switcher/content-switcher.component.tsx

@@ -1,18 +1,18 @@
 import React, { useEffect, useMemo, useState } from 'react';
 import classNames from 'classnames';
-import { FormGroup, ContentSwitcher, Switch } from '@carbon/react';
+import { FormGroup, ContentSwitcher as Switcher, Switch } from '@carbon/react';


We should alias this to something like RfeContentSwitcher to avoid any potential namespace conflicts with the Carbon Switcher component

Or CdsContentSwitcher

denniskigen · 2024-04-24T16:57:38Z

src/components/inputs/ui-select-extended/ui-select-extended.component.tsx

 import styles from './ui-select-extended.scss';

-const UISelectExtended: React.FC<OHRIFormFieldProps> = ({ question, handler, onChange, previousValue }) => {
+const UISelectExtended: React.FC<FormFieldProps> = ({ question, handler, onChange, previousValue }) => {


Suggested change

const UISelectExtended: React.FC<FormFieldProps> = ({ question, handler, onChange, previousValue }) => {

const UiSelectExtended: React.FC<FormFieldProps> = ({ question, handler, onChange, previousValue }) => {

pirupius · 2024-04-24T17:22:07Z

Hi @ibacher would love if you have some comments or suggestions on what naming we can improve and will submit those in follow up PR's so as to unblock other PR's

ibacher

Renaming seems fine, so here's a smattering of random suggestions 😄. Keep up the good work!

ibacher · 2024-04-24T17:55:32Z

src/api/api.ts

@@ -121,7 +121,7 @@ export async function fetchOpenMRSForm(nameOrUUID: string): Promise<OpenmrsForm
 * @param {OpenmrsForm} form - The OpenMRS form object.
 * @returns {Promise<any | null>} - A Promise that resolves to the fetched ClobData or null if not found.
 */
-export async function fetchClobData(form: OpenmrsForm): Promise<any | null> {
+export async function fetchClobdata(form: OpenmrsForm): Promise<any | null> {


It would be nice to have a better type here or to mark the return type as Promise<unknown | null>. It's more annoying, but hey. Even Record<string, unknown> is substantially better.

With the o3 module in place, we can as well consider getting rid of this function.

ibacher · 2024-04-24T17:57:40Z

src/components/inputs/content-switcher/content-switcher.component.tsx

@@ -1,18 +1,18 @@
 import React, { useEffect, useMemo, useState } from 'react';
 import classNames from 'classnames';
-import { FormGroup, ContentSwitcher, Switch } from '@carbon/react';
+import { FormGroup, ContentSwitcher as Switcher, Switch } from '@carbon/react';


Or CdsContentSwitcher

ibacher · 2024-04-24T18:01:30Z

src/components/inputs/multi-select/multi-select.component.tsx

@@ -51,7 +51,7 @@ export const OHRIMultiSelect: React.FC<OHRIFormFieldProps> = ({ question, onChan
    }));

  const initiallySelectedQuestionItems = [];
-  questionItems.forEach((item) => {
+  questionItems.forEach(item => {


Usually, I'm all in favour of fore-going React hooks, but from the name here, it seems like both questionItems and this could benefit from useMemo(), since we only really need to recalculate them if the question changes.

src/components/inputs/tooltip/tooltip.component.tsx

ibacher · 2024-04-24T18:05:14Z

src/components/inputs/workspace-launcher/workspace-launcher.component.tsx

@@ -3,10 +3,10 @@ import { useTranslation } from 'react-i18next';
 import { showSnackbar } from '@openmrs/esm-framework';
 import { useLaunchWorkspaceRequiringVisit } from '@openmrs/esm-patient-common-lib';


Now that the workspace stuff has been integrated into the framework, we can hopefully drop the need for depending on the patient-common-lib here.

Thanks for the heads up. I've created this ticket to handle this change as we wait for any other new developments that might come as part of this patient chart PR

src/components/value/view/field-value-view.component.tsx

src/hooks/useClobData.tsx

ibacher · 2024-04-24T18:18:21Z

src/registry/registry.ts

@@ -249,7 +249,7 @@ export function getRegisteredExpressionHelpers() {
 }

 function getFormsStore(): FormsRegistryStoreState {
-  return getGlobalStore<FormsRegistryStoreState>(OHRIFormsStore, {
+  return getGlobalStore<FormsRegistryStoreState>(FormsStore, {


This is a weird way of using a global store. Normally, we'd create a store in a variable like:

const formsStore = createGlobalStore<FormsRegistryStoreState>(FormsStore, /* ... */);

Then, you could create synchronous handlers, e.g.,

export function getRegisteredExpressionHelpers() { return formsStore.getState().expressionHelpers; }

But, even better, since this is React, you can just turn everything into hooks:

export const useRegisteredExpressionHelpers = useStore(formsStore, (state) => state.expressionHelpers);

Setters can then be implemented more sanely like this:

export function registerFieldValidator(registration: ComponentRegistration<FormFieldValidator>) { formsStore.set((state) => state.fieldValidators.push(registration)); }

src/submission-handlers/base-handlers.ts

ibacher · 2024-04-24T18:21:36Z

src/submission-handlers/base-handlers.ts

+      uuid: encounter.location.uuid,
+    };
+  },
+  getDisplayValue: (field: FormField, value) => {


Ditto:

getDisplayValue: (field: FormField, value: { display: string })

pirupius force-pushed the refactor/rename-files branch 3 times, most recently from 78d2a32 to 82ec502 Compare April 24, 2024 10:02

pirupius commented Apr 24, 2024

View reviewed changes

pirupius marked this pull request as ready for review April 24, 2024 14:12

pirupius requested review from samuelmale and denniskigen April 24, 2024 14:12

pirupius force-pushed the refactor/rename-files branch from a5cf8bd to 3384a6a Compare April 24, 2024 14:38

pirupius added 5 commits April 24, 2024 19:26

(BREAKING) Rename files and reorganize styles

8d84a06

rebase cleanups reference refactors

more refactors

7ec2b64

more clean ups

1aa8ffc

forms folder changes and aligns styles

d8b8f74

rebase fixes

c053147

pirupius force-pushed the refactor/rename-files branch from 71fa689 to c053147 Compare April 24, 2024 16:35

more clean ups

bce6acf

denniskigen approved these changes Apr 24, 2024

View reviewed changes

denniskigen requested review from ibacher and denniskigen April 24, 2024 17:08

corrects rebase residue

4d4a936

pirupius merged commit 8acbb35 into main Apr 24, 2024
4 checks passed

pirupius deleted the refactor/rename-files branch April 24, 2024 17:20

ibacher reviewed Apr 24, 2024

View reviewed changes

This was referenced Apr 24, 2024

(chore) O3-3114: Bump react form engine openmrs/openmrs-esm-patient-chart#1809

Merged

(chore) Bump form engine library and refactor type and component imports openmrs/openmrs-esm-form-builder#257

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

(BREAKING) O3-3037 : Refactor components and rename files #211

(BREAKING) O3-3037 : Refactor components and rename files #211

pirupius commented Apr 24, 2024 •

edited

Loading

pirupius Apr 24, 2024

github-actions bot commented Apr 24, 2024 •

edited

Loading

denniskigen left a comment

denniskigen Apr 24, 2024

ibacher Apr 24, 2024

denniskigen Apr 24, 2024

pirupius commented Apr 24, 2024

ibacher left a comment

ibacher Apr 24, 2024

samuelmale Apr 24, 2024

ibacher Apr 24, 2024

ibacher Apr 24, 2024

ibacher Apr 24, 2024

pirupius Apr 25, 2024

ibacher Apr 24, 2024

ibacher Apr 24, 2024

	const UISelectExtended: React.FC<FormFieldProps> = ({ question, handler, onChange, previousValue }) => {
	const UiSelectExtended: React.FC<FormFieldProps> = ({ question, handler, onChange, previousValue }) => {

		@@ -3,10 +3,10 @@ import { useTranslation } from 'react-i18next';
		import { showSnackbar } from '@openmrs/esm-framework';
		import { useLaunchWorkspaceRequiringVisit } from '@openmrs/esm-patient-common-lib';

(BREAKING) O3-3037 : Refactor components and rename files #211

(BREAKING) O3-3037 : Refactor components and rename files #211

Conversation

pirupius commented Apr 24, 2024 • edited Loading

Requirements

Summary

Screenshots

Related Issue

Other

Choose a reason for hiding this comment

github-actions bot commented Apr 24, 2024 • edited Loading

denniskigen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pirupius commented Apr 24, 2024

ibacher left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pirupius commented Apr 24, 2024 •

edited

Loading

github-actions bot commented Apr 24, 2024 •

edited

Loading