-
Notifications
You must be signed in to change notification settings - Fork 75
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
feat: update codelist config design #14276
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #14276 +/- ##
=======================================
Coverage 95.57% 95.58%
=======================================
Files 1864 1867 +3
Lines 24151 24210 +59
Branches 2786 2787 +1
=======================================
+ Hits 23082 23140 +58
- Misses 812 814 +2
+ Partials 257 256 -1 ☔ View full report in Codecov by Sentry. |
…during onBlurAny and onClose no longer contains any logic.
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice cleanup, but I have some suggestions 🤗
...ditOptions/OptionTabs/EditTab/OptionListEditor/LibraryOptionsEditor/LibraryOptionsEditor.tsx
Show resolved
Hide resolved
...ackages/ux-editor/src/components/config/editModal/EditOptions/OptionTabs/OptionTabs.test.tsx
Show resolved
Hide resolved
...onents/config/editModal/EditOptions/OptionTabs/EditTab/OptionListEditor/OptionListEditor.tsx
Outdated
Show resolved
Hide resolved
...ditOptions/OptionTabs/EditTab/OptionListEditor/LibraryOptionsEditor/LibraryOptionsEditor.tsx
Outdated
Show resolved
Hide resolved
...ditOptions/OptionTabs/EditTab/OptionListEditor/LibraryOptionsEditor/LibraryOptionsEditor.tsx
Outdated
Show resolved
Hide resolved
...ditOptions/OptionTabs/EditTab/OptionListEditor/LibraryOptionsEditor/LibraryOptionsEditor.tsx
Outdated
Show resolved
Hide resolved
...ackages/ux-editor/src/components/config/editModal/EditOptions/OptionTabs/EditTab/EditTab.tsx
Outdated
Show resolved
Hide resolved
...es/ux-editor/src/components/config/editModal/EditOptions/OptionTabs/EditTab/EditTab.test.tsx
Outdated
Show resolved
Hide resolved
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.movAlso, when I have connected an optionlist in the Skjermopptak.2024-12-13.kl.12.53.07.movAnd 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. |
.../ux-editor/src/components/config/editModal/EditOptions/OptionTabs/utils/optionsUtils.test.ts
Outdated
Show resolved
Hide resolved
.../ux-editor/src/components/config/editModal/EditOptions/OptionTabs/utils/optionsUtils.test.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Testing:
- Immediately closing the modal when on a component options list should return us back to the original view
- Text in modal that lists code lists from library was larger than the modal`s heading
- Code list name text was larger than the image component name text
- Small adjustments to spacing
We fixed this in pair programming 😊
Description
Update the design of the codelist section of our config page. Earlier discuessed in another PR
Before
After
Related Issue(s)
Verification