feat: show code list title as readOnly displayTile if in use #14400
Conversation
standeren
added
skip-releasenotes
github-actions
bot
added
the
solution/studio/designer
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #14400 +/- ##
=======================================
Coverage 95.66% 95.66%
=======================================
Files 1891 1891
Lines 24581 24583 +2
Branches 2822 2823 +1
=======================================
+ Hits 23515 23517 +2
Misses 805 805
Partials 261 261 ☔ View full report in Codecov by Sentry. |
standeren
changed the title
📝 WalkthroughWalkthroughThis pull request introduces changes to the code list editing functionality in a frontend application. Modifications include adding a Norwegian error message for disabled code list editing, updating a test case to verify the rendering behavior of the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (4)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
standeren
force-pushed
the
disallow-code-list-id-change-when-in-use
branch
from
b8dbe82 to
fd9b3c8
Compare
...ry/src/ContentLibrary/LibraryBody/pages/CodeListPage/CodeLists/EditCodeList/EditCodeList.tsx
Outdated
Show resolved
Hide resolved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
frontend/libs/studio-content-library/src/ContentLibrary/LibraryBody/pages/CodeListPage/CodeLists/EditCodeList/EditCodeList.tsx (1)
68-124: Well-implemented component with clean conditional rendering!The component handles both editable and non-editable states elegantly. Good use of StudioToggleableTextfield and StudioDisplayTile components.
Consider extracting validation error messages to a separate utility function for better reusability:
+const getCodeListValidationError = (newCodeListId: string, invalidCodeListNames: string[]) => { + const fileNameError = FileNameUtils.findFileNameError(newCodeListId, invalidCodeListNames); + return getInvalidInputFileNameErrorMessage(fileNameError); +}; const handleValidateCodeListId = (newCodeListId: string) => { const invalidCodeListNames = ArrayUtils.removeItemByValue(codeListNames, codeListTitle); - const fileNameError = FileNameUtils.findFileNameError(newCodeListId, invalidCodeListNames); - return getInvalidInputFileNameErrorMessage(fileNameError); + return getCodeListValidationError(newCodeListId, invalidCodeListNames); };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
frontend/language/src/nb.json(1 hunks)frontend/libs/studio-content-library/src/ContentLibrary/LibraryBody/pages/CodeListPage/CodeLists/CodeLists.test.tsx(1 hunks)frontend/libs/studio-content-library/src/ContentLibrary/LibraryBody/pages/CodeListPage/CodeLists/EditCodeList/EditCodeList.tsx(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Build environment and run e2e test
- GitHub Check: Testing
- GitHub Check: Typechecking and linting
- GitHub Check: CodeQL
🔇 Additional comments (4)
frontend/libs/studio-content-library/src/ContentLibrary/LibraryBody/pages/CodeListPage/CodeLists/EditCodeList/EditCodeList.tsx (3)
47-47: LGTM! Clean variable extraction.The boolean variable clearly expresses the editability state based on code list usage, following the suggested extraction from previous reviews.
51-56: Well-structured component extraction!The EditCodeListTitle component extraction improves code organization and follows the single responsibility principle. Props interface is clean and focused.
125-130: LGTM! Clean type update.The type change from StudioComponentsCodeList to CodeList aligns with the import consolidation without affecting functionality.
frontend/libs/studio-content-library/src/ContentLibrary/LibraryBody/pages/CodeListPage/CodeLists/CodeLists.test.tsx (1)
179-193: Well-written test case covering the new functionality!The test properly verifies that a display tile is rendered instead of an edit button when the code list is in use. Good assertions checking both presence and role attributes.
standeren
force-pushed
the
disallow-code-list-id-change-when-in-use
branch
from
719df4a to
d5b5374
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
frontend/libs/studio-content-library/src/ContentLibrary/LibraryBody/pages/CodeListPage/CodeLists/EditCodeList/EditCodeList.tsx (1)
100-102: Consider adding error handling for the onBlur event.The onBlur handler should consider handling potential errors from the validation before updating the ID.
const handleUpdateCodeListId = (newCodeListId: string) => { - if (newCodeListId !== codeListTitle) onUpdateCodeListId(codeListTitle, newCodeListId); + const validationError = handleValidateCodeListId(newCodeListId); + if (!validationError && newCodeListId !== codeListTitle) { + onUpdateCodeListId(codeListTitle, newCodeListId); + } };Also applies to: 120-120
frontend/libs/studio-content-library/src/ContentLibrary/LibraryBody/pages/CodeListPage/CodeLists/CodeLists.tsx (1)
Line range hint
119-125: Consider extracting error state into a separate component.For improved modularity, consider extracting the error state rendering into a separate component. This would make the code more maintainable and reusable.
+const CodeListError: React.FC = () => { + const { t } = useTranslation(); + return ( + <StudioAlert size='small' severity='danger'> + {t('app_content_library.code_lists.fetch_error')} + </StudioAlert> + ); +}; function CodeListAccordionContent({ codeListData, codeListSources, ...rest }: CodeListAccordionContentProps): React.ReactElement { - const { t } = useTranslation(); return ( <Accordion.Content> - {codeListData.hasError ? ( - <StudioAlert size='small' severity='danger'> - {t('app_content_library.code_lists.fetch_error')} - </StudioAlert> - ) : ( + {codeListData.hasError ? <CodeListError /> : ( <EditCodeList codeList={codeListData.data} codeListTitle={codeListData.title}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
frontend/language/src/nb.json(1 hunks)frontend/libs/studio-content-library/src/ContentLibrary/LibraryBody/pages/CodeListPage/CodeLists/CodeLists.test.tsx(1 hunks)frontend/libs/studio-content-library/src/ContentLibrary/LibraryBody/pages/CodeListPage/CodeLists/CodeLists.tsx(2 hunks)frontend/libs/studio-content-library/src/ContentLibrary/LibraryBody/pages/CodeListPage/CodeLists/EditCodeList/EditCodeList.tsx(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- frontend/libs/studio-content-library/src/ContentLibrary/LibraryBody/pages/CodeListPage/CodeLists/CodeLists.test.tsx
- frontend/language/src/nb.json
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Build environment and run e2e test
- GitHub Check: Testing
- GitHub Check: CodeQL
🔇 Additional comments (7)
frontend/libs/studio-content-library/src/ContentLibrary/LibraryBody/pages/CodeListPage/CodeLists/EditCodeList/EditCodeList.tsx (5)
5-5: LGTM!The StudioDisplayTile import is correctly placed and properly utilized in the new component.
52-52: Consider renaming the variable to match the previous review suggestion.For consistency with the previous review suggestion and better clarity, consider renaming
isCodeListEditabletoisCodeListIdEditableas it specifically controls the ID field's editability.
56-61: LGTM! Clean component extraction.The EditCodeListTitle component extraction improves code organization and maintainability. All necessary props are correctly passed.
84-89: LGTM! Well-defined props interface.The EditCodeListTitleProps type is clear and properly typed.
110-139: LGTM! Clean conditional rendering implementation.The component appropriately switches between editable and read-only views based on the isCodeListEditable flag. The translation keys and props are handled correctly in both cases.
frontend/libs/studio-content-library/src/ContentLibrary/LibraryBody/pages/CodeListPage/CodeLists/CodeLists.tsx (2)
114-114: LGTM! Prop addition is well-structured.The addition of
codeListSourcesprop is properly typed and aligns with the PR's objective to handle code lists that are in use.
126-131: LGTM! Component refactoring improves readability.The
EditCodeListcomponent changes maintain proper prop passing while improving code readability through multi-line formatting. The error handling remains intact, and the newcodeListSourcesprop is correctly forwarded.
Gjerne lag issue for det første punktet! 🥳 |
Så bra! @coderabbitai create a new issue regarding |
|
Tip For best results, initiate chat on the files or code changes. @ErlingHauan I've created issue #14454 to track the Enter key support enhancement for 🔗 Analysis chain🏁 Scripts executedThe following scripts were executed for the analysis: Script: Length of output: 709 |
Description
Show title as
readOnlydisplayTileif in use.Verification
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Refactor
Tests