feat: update codelist config design #14276

Konrad-Simso · 2024-12-12T15:19:09Z

Description

Update the design of the codelist section of our config page. Earlier discuessed in another PR

Before

After

Related Issue(s)

Merge "select codelist" and "edit codelist" views #13685

Verification

Your code builds clean without any errors or warnings
Manual testing done (required)
Relevant automated test added (if you find this hard, leave it and we'll help out)

…nceId is used

codecov · 2024-12-12T15:30:30Z

Codecov Report

Attention: Patch coverage is 98.33333% with 1 line in your changes missing coverage. Please review.

Project coverage is 95.53%. Comparing base (37d92b5) to head (cf2ebbd).
Report is 9 commits behind head on main.

Files with missing lines	Patch %	Lines
...Editor/ManualOptionsEditor/ManualOptionsEditor.tsx	94.44%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #14276   +/-   ##
=======================================
  Coverage   95.52%   95.53%           
=======================================
  Files        1826     1830    +4     
  Lines       23757    23772   +15     
  Branches     2752     2752           
=======================================
+ Hits        22695    22710   +15     
  Misses        807      807           
  Partials      255      255

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

…during onBlurAny and onClose no longer contains any logic.

Konrad-Simso · 2024-12-13T11:29:34Z

Sorry for the late commit, i saw it when doing review of your code @standeren in this PR and thought it could be changed here as your PR is not related to the component itself

standeren

Nice cleanup, but I have some suggestions 🤗

standeren · 2024-12-13T08:02:27Z

frontend/language/src/nb.json

@@ -1560,7 +1560,7 @@
  "ux_editor.modal_new_option": "Legg til alternativ",
  "ux_editor.modal_properties_add_radio_button_options": "Hvordan vil du legge til radioknapper?",
  "ux_editor.modal_properties_code_list": "Velg fra biblioteket",
-  "ux_editor.modal_properties_code_list_alert_title": "Du redigerer nå en kodeliste i biblioteket.",
+  "ux_editor.modal_properties_code_list_alert_title": "Du er i ferd med å endre en kodeliste i biblioteket. Da endrer du også kodelisten for alle andre steder der den blir brukt.",


Suggested change

"ux_editor.modal_properties_code_list_alert_title": "Du er i ferd med å endre en kodeliste i biblioteket. Da endrer du også kodelisten for alle andre steder der den blir brukt.",

"ux_editor.modal_properties_code_list_alert_title": "Du er i ferd med å endre en kodeliste i biblioteket. Da endrer du også kodelisten alle andre steder der den blir brukt.",

standeren · 2024-12-13T08:12:55Z

frontend/language/src/nb.json

  "ux_editor.options.multiple": "{{value}} alternativer",
  "ux_editor.options.option_edit_text": "Rediger kodeliste",
  "ux_editor.options.option_remove_text": "Fjern valgt kodeliste",
  "ux_editor.options.section_heading": "Valg for kodelister",
  "ux_editor.options.single": "{{value}} alternativ",
  "ux_editor.options.tab_code_list": "Velg kodeliste",
+  "ux_editor.options.tab_code_list_alert_title": "Kan ikke åpne Velg kodeliste fordi det er oppgitt en ID på fanen Angi referanse-ID. Fjern denne referansen og gå tilbake for å velge kodeliste fra biblioteket.",


Should this text key maybe be ux_editor.options.tab_option_list_alert_title ?

standeren · 2024-12-13T09:33:58Z

...ditOptions/OptionTabs/EditTab/OptionListEditor/LibraryOptionsEditor/LibraryOptionsEditor.tsx

+} from '@studio/components';
+import type { CodeListEditorTexts } from '@studio/components';
+import { PencilIcon, TrashIcon } from '@studio/icons';
+import { usePreviewContext } from 'app-development/contexts/PreviewContext';


This one should not be imported from app-development. But we can create a separate PR that moves the usePreviewContext into shared 👍

standeren · 2024-12-13T10:14:31Z

...ackages/ux-editor/src/components/config/editModal/EditOptions/OptionTabs/OptionTabs.test.tsx

I know this is not a part of your changes, but I still think it would be relevant to include in this PR;
I noticed that handleComponentChange has a default value jest.fn() when it should be defined in the top as a mock. And then I also discovered that there is no test that tests this property, which it should be 😄

Hmm, in OptionTabs.tsx handleComponentChange is passed along to the child components and not used or altered. Which is a pattern in some files, could it be an option to make a context higher up in the structure to not require prop drilling as much?
It eventually goes all the way up to Text.tsx, but placing the context there would make the context very large

standeren · 2024-12-13T11:15:42Z

...onents/config/editModal/EditOptions/OptionTabs/EditTab/OptionListEditor/OptionListEditor.tsx

          />
        );
+      } else {


All these conditions should not be in the success case from the optionListQuery as only the LibraryOptionEditor is depending on it.

standeren · 2024-12-13T11:33:42Z

...ditOptions/OptionTabs/EditTab/OptionListEditor/LibraryOptionsEditor/LibraryOptionsEditor.tsx

+  const codeListLabels: string = localOptionList
+    .slice(0, 3)
+    .map((option: Option) => `${option.label}`)
+    .join(' | ');


Same comment here as for the manualOtpions

standeren · 2024-12-13T11:35:48Z

...ditOptions/OptionTabs/EditTab/OptionListEditor/LibraryOptionsEditor/LibraryOptionsEditor.tsx

+  const { org, app } = useStudioEnvironmentParams();
+  const { doReloadPreview } = usePreviewContext();
+  const { mutate: updateOptionList } = useUpdateOptionListMutation(org, app);
+  const [localOptionList, setLocalOptionList] = useState<Option[]>(optionsList);


Is this state necessary?
And nitpicking, but it should be [localOptionsList, setLocalOptionsList] as in the variable in the initial state 🙈

After testing, likely no it is no longer needed

standeren · 2024-12-13T11:36:14Z

...ditOptions/OptionTabs/EditTab/OptionListEditor/LibraryOptionsEditor/LibraryOptionsEditor.tsx

+import { usePreviewContext } from 'app-development/contexts/PreviewContext';
+import { useStudioEnvironmentParams } from 'app-shared/hooks/useStudioEnvironmentParams';
+import { useUpdateOptionListMutation } from 'app-shared/hooks/mutations';
+import { useOptionListEditorTexts } from '@altinn/ux-editor/components/config/editModal/EditOptions/OptionTabs/hooks';


Should be relative import 🙈

standeren · 2024-12-13T11:39:28Z

...ackages/ux-editor/src/components/config/editModal/EditOptions/OptionTabs/EditTab/EditTab.tsx

        optionsId={component.optionsId}
        component={component}
        handleComponentChange={handleComponentChange}
+        setComponentHasOptionList={setComponentHasOptionList}


If this value is defined by the incoming components fields, should we not be able to evaluate this in a const used only where it is needed? And any changes to the component that would affect this value would anyway be noticed due to rerendring affected by changing the component-prop 🤔

I added the useState to make sure we continue to render the modals while the user is making changes. Removing it results in the modals being closed because they are no longer being rendered as they get replaced with SelectedOptionList

If you have a better idea for how to fix this, then i'm happy to change it

standeren · 2024-12-13T11:42:43Z

...es/ux-editor/src/components/config/editModal/EditOptions/OptionTabs/EditTab/EditTab.test.tsx

@@ -18,7 +18,7 @@ describe('EditTab', () => {
  it('should render DisplayChosenOption', async () => {
    renderEditTab();
    expect(
-      screen.getByRole('button', { name: textMock('ux_editor.options.option_remove_text') }),
+      await screen.findByText(textMock('ux_editor.modal_properties_code_list_custom_list')),


Why does this test have to be awaited and not use getByText? If there is due to some data needed from queries, it is better to adapt the tests to test for the spinner being there when waiting and for the data being there when the required data is on the cache.

thank you for spotting it!

standeren · 2024-12-13T11:53:25Z

And I also have a general comment concerning the delete-functionality, as well as the conflicted-sources (trying to set a library id if manual options are set and vice versa).

We should sync the design here along with the image-component. For the image component the delete-choices are separated between deleting the ref and deleting the actual source.

When there are conflicting sources it is visualized like this in the image component:

Skjermopptak.2024-12-13.kl.09.11.34.mov

Also, when I have connected an optionlist in the Velg kodeliste I get the same codelist id visualized in the Ani referanse-id:

Skjermopptak.2024-12-13.kl.12.53.07.mov

And one last small thing I noticed, when I have a faulty codeList I cant remove it. Might be that this can be fixed easier when the other PR that fetches only the specified code list is implemented tho.

Konrad-Simso added 5 commits December 11, 2024 14:41

Redesign

9f81a8c

Refactor OptionListEditor and render alert in EditTab when refere…

511f9cd

…nceId is used

Remove unused line

09d4f8e

Fix imports and tests

6eff9ac

Remove unused line in css

1c80c47

github-actions bot added area/ui-editor Area: Related to the designer tool for assembling app UI in Altinn Studio. solution/studio/designer Issues related to the Altinn Studio Designer solution. frontend labels Dec 12, 2024

Konrad-Simso linked an issue Dec 12, 2024 that may be closed by this pull request

Merge "select codelist" and "edit codelist" views #13685

Open

standeren self-assigned this Dec 13, 2024

Remove superfluous code, the code is no longer needed as we now save …

60f3bd5

…during onBlurAny and onClose no longer contains any logic.

standeren requested changes Dec 13, 2024

View reviewed changes

standeren assigned Konrad-Simso and unassigned standeren Dec 13, 2024

Konrad-Simso added 5 commits December 13, 2024 15:03

Updates from review

212598c

Remove doubled css

7e712e8

Remove import

f208e97

Rename css class

8f9e789

Add alert in ReferenceTab.tsx

cf2ebbd

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: update codelist config design #14276

feat: update codelist config design #14276

Konrad-Simso commented Dec 12, 2024

codecov bot commented Dec 12, 2024 •

edited

Loading

Konrad-Simso commented Dec 13, 2024 •

edited

Loading

standeren left a comment

standeren Dec 13, 2024

standeren Dec 13, 2024

standeren Dec 13, 2024

standeren Dec 13, 2024

Konrad-Simso Dec 13, 2024

standeren Dec 13, 2024

standeren Dec 13, 2024

standeren Dec 13, 2024

Konrad-Simso Dec 13, 2024

standeren Dec 13, 2024

standeren Dec 13, 2024

Konrad-Simso Dec 13, 2024

standeren Dec 13, 2024

Konrad-Simso Dec 13, 2024

standeren commented Dec 13, 2024 •

edited

Loading

	"ux_editor.modal_properties_code_list_alert_title": "Du er i ferd med å endre en kodeliste i biblioteket. Da endrer du også kodelisten for alle andre steder der den blir brukt.",
	"ux_editor.modal_properties_code_list_alert_title": "Du er i ferd med å endre en kodeliste i biblioteket. Da endrer du også kodelisten alle andre steder der den blir brukt.",

feat: update codelist config design #14276

Are you sure you want to change the base?

feat: update codelist config design #14276

Conversation

Konrad-Simso commented Dec 12, 2024

Description

Before

After

Related Issue(s)

Verification

codecov bot commented Dec 12, 2024 • edited Loading

Codecov Report

Konrad-Simso commented Dec 13, 2024 • edited Loading

standeren 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

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

standeren commented Dec 13, 2024 • edited Loading

codecov bot commented Dec 12, 2024 •

edited

Loading

Konrad-Simso commented Dec 13, 2024 •

edited

Loading

standeren commented Dec 13, 2024 •

edited

Loading