fix: Add text resources to create code list dialog #14646
Conversation
📝 WalkthroughWalkthroughThe update refactors text resource management across several studio components. The Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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 (
|
github-actions
bot
added
solution/studio/designer
…-create-code-list-dialog # Conflicts: # frontend/libs/studio-content-library/src/ContentLibrary/LibraryBody/pages/CodeListPage/CodeListPage.tsx # frontend/libs/studio-content-library/src/ContentLibrary/LibraryBody/pages/CodeListPage/CodeLists/CodeLists.tsx # frontend/libs/studio-content-library/src/ContentLibrary/LibraryBody/pages/CodeListPage/utils/utils.test.ts
TomasEng
force-pushed
the
add-text-resources-to-create-code-list-dialog
branch
from
4871a9e to
9fd63fd
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #14646 +/- ##
==========================================
- Coverage 95.75% 95.75% -0.01%
==========================================
Files 1913 1912 -1
Lines 24925 24924 -1
Branches 2848 2848
==========================================
- Hits 23868 23867 -1
Misses 799 799
Partials 258 258 ☔ View full report in Codecov by Sentry. |
TomasEng
added
skip-releasenotes
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
TomasEng
added
the
area/content-library
![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 (4)
frontend/libs/studio-content-library/src/ContentLibrary/LibraryBody/pages/CodeListPage/utils/utils.ts (1)
40-48: Add JSDoc documentation for the new utility functions.While the functions are well-implemented, adding JSDoc documentation would improve maintainability by clearly describing:
- Purpose and usage of each function
- Parameter types and their significance
- Return value types and possible undefined cases
Example documentation:
+/** + * Retrieves text resources for a specific language + * @param language - The language code to filter text resources + * @param textResources - Optional object containing text resources by language + * @returns Array of text resources for the specified language or undefined if not found + */ export const getTextResourcesForLanguage = ( language: string, textResources?: TextResources, ): TextResource[] | undefined => textResources?.[language]; +/** + * Creates a TextResourceWithLanguage object by combining a language code with a text resource + * @param language - The language code to associate with the text resource + * @param textResource - The text resource to be wrapped + * @returns Combined object containing both language and text resource + */ export const createTextResourceWithLanguage = ( language: string, textResource: TextResource, ): TextResourceWithLanguage => ({ language, textResource });frontend/libs/studio-content-library/src/ContentLibrary/LibraryBody/pages/CodeListPage/CodeListPage.tsx (1)
67-67: Consider extracting the hardcoded language value.The language code 'nb' is hardcoded in two places. Consider extracting it to a named constant at the top of the file until the referenced issue #14572 is implemented.
+const DEFAULT_LANGUAGE = 'nb'; + export function CodeListPage({ // ... }) { // ... const handleChangeTextResource = useCallback( (textResource: TextResource) => { - const updatedTextResource = createTextResourceWithLanguage('nb', textResource); + const updatedTextResource = createTextResourceWithLanguage(DEFAULT_LANGUAGE, textResource); onUpdateTextResource?.(updatedTextResource); }, [onUpdateTextResource], ); // ... } -const language: string = 'nb'; // Todo: Let the user choose the language: https://github.com/Altinn/altinn-studio/issues/14572 +const language: string = DEFAULT_LANGUAGE; // Todo: Let the user choose the language: https://github.com/Altinn/altinn-studio/issues/14572Also applies to: 112-112
frontend/libs/studio-content-library/src/ContentLibrary/LibraryBody/pages/CodeListPage/utils/utils.test.ts (1)
167-174: Consider adding edge case tests.While the basic functionality is tested, consider adding tests for edge cases such as empty strings or special characters in the language parameter.
describe('createTextResourceWithLanguage', () => { it('Creates a TextResourceWithLanguage object from the parameters', () => { const language = 'nb'; const textResource = label1ResourceNb; const expectedResult: TextResourceWithLanguage = { language, textResource }; expect(createTextResourceWithLanguage(language, textResource)).toEqual(expectedResult); }); + it('Handles empty language string', () => { + const language = ''; + const textResource = label1ResourceNb; + const expectedResult: TextResourceWithLanguage = { language, textResource }; + expect(createTextResourceWithLanguage(language, textResource)).toEqual(expectedResult); + }); + it('Handles special characters in language', () => { + const language = 'zh-CN'; + const textResource = label1ResourceNb; + const expectedResult: TextResourceWithLanguage = { language, textResource }; + expect(createTextResourceWithLanguage(language, textResource)).toEqual(expectedResult); + }); });frontend/libs/studio-content-library/src/ContentLibrary/LibraryBody/pages/CodeListPage/CodeListPage.test.tsx (1)
259-291: Consider consolidating dialog interaction utilities.The utility functions are well-organized but could benefit from being grouped into a test helper class/object for better maintainability.
+class DialogTestHelper { + static async openCreateDialog(user: UserEvent): Promise<HTMLElement> { + const createButtonLabel = textMock('app_content_library.code_lists.create_new_code_list'); + await user.click(screen.getByRole('button', { name: createButtonLabel })); + return screen.getByRole('dialog'); + } + + static async addCodeListItem(user: UserEvent, area: HTMLElement): Promise<void> { + const addButtonLabel = textMock('code_list_editor.add_option'); + await user.click(within(area).getByRole('button', { name: addButtonLabel })); + } + // ... other methods +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
frontend/libs/studio-components/src/components/StudioTextResourceInput/StudioTextResourceInput.tsx(2 hunks)frontend/libs/studio-content-library/src/ContentLibrary/LibraryBody/pages/CodeListPage/CodeListPage.test.tsx(4 hunks)frontend/libs/studio-content-library/src/ContentLibrary/LibraryBody/pages/CodeListPage/CodeListPage.tsx(3 hunks)frontend/libs/studio-content-library/src/ContentLibrary/LibraryBody/pages/CodeListPage/CodeLists/CodeLists.tsx(1 hunks)frontend/libs/studio-content-library/src/ContentLibrary/LibraryBody/pages/CodeListPage/CodeLists/EditCodeList/EditCodeList.tsx(3 hunks)frontend/libs/studio-content-library/src/ContentLibrary/LibraryBody/pages/CodeListPage/CodeLists/EditCodeList/utils.test.ts(0 hunks)frontend/libs/studio-content-library/src/ContentLibrary/LibraryBody/pages/CodeListPage/CodeLists/EditCodeList/utils.ts(0 hunks)frontend/libs/studio-content-library/src/ContentLibrary/LibraryBody/pages/CodeListPage/CodeListsActionsBar/CodeListsActionsBar.tsx(3 hunks)frontend/libs/studio-content-library/src/ContentLibrary/LibraryBody/pages/CodeListPage/CodeListsActionsBar/CreateNewCodeListModal/CreateNewCodeListModal.tsx(5 hunks)frontend/libs/studio-content-library/src/ContentLibrary/LibraryBody/pages/CodeListPage/utils/utils.test.ts(3 hunks)frontend/libs/studio-content-library/src/ContentLibrary/LibraryBody/pages/CodeListPage/utils/utils.ts(2 hunks)frontend/libs/studio-content-library/src/index.ts(1 hunks)
💤 Files with no reviewable changes (2)
- frontend/libs/studio-content-library/src/ContentLibrary/LibraryBody/pages/CodeListPage/CodeLists/EditCodeList/utils.ts
- frontend/libs/studio-content-library/src/ContentLibrary/LibraryBody/pages/CodeListPage/CodeLists/EditCodeList/utils.test.ts
🔇 Additional comments (10)
frontend/libs/studio-content-library/src/index.ts (1)
8-10: LGTM! Clean and focused type exports.The new type exports are well-organized and align with the PR's objective to enhance text resource management.
frontend/libs/studio-content-library/src/ContentLibrary/LibraryBody/pages/CodeListPage/CodeListsActionsBar/CodeListsActionsBar.tsx (1)
13-20: LGTM! Well-structured prop additions and component integration.The new optional props are properly typed and consistently passed to child components, maintaining good component composition.
Also applies to: 57-62
frontend/libs/studio-content-library/src/ContentLibrary/LibraryBody/pages/CodeListPage/CodeListPage.tsx (1)
60-63: LGTM! Well-implemented memoization and callback handling.The
textResourcesForLanguageandhandleChangeTextResourceare properly memoized with correct dependencies, preventing unnecessary re-renders.Also applies to: 65-71
frontend/libs/studio-content-library/src/ContentLibrary/LibraryBody/pages/CodeListPage/CodeLists/CodeLists.tsx (1)
10-10: LGTM! Props type updated for consistent text resource handling.The changes to
CodeListsPropsalign with the standardized approach to text resource management across components.Also applies to: 14-15, 21-21
frontend/libs/studio-content-library/src/ContentLibrary/LibraryBody/pages/CodeListPage/CodeListsActionsBar/CreateNewCodeListModal/CreateNewCodeListModal.tsx (1)
8-8: LGTM! Text resource handling properly implemented.The changes correctly implement text resource management in the create dialog, with proper prop drilling through the component hierarchy.
Also applies to: 18-18, 21-21, 25-26, 28-29, 53-54, 56-57, 66-67, 69-70, 75-76, 78-79, 138-139, 141-142
frontend/libs/studio-content-library/src/ContentLibrary/LibraryBody/pages/CodeListPage/utils/utils.test.ts (1)
2-15: LGTM! Comprehensive test coverage for text resource utilities.The test cases thoroughly cover the functionality of
getTextResourcesForLanguage, including edge cases and error conditions.Also applies to: 153-165
frontend/libs/studio-content-library/src/ContentLibrary/LibraryBody/pages/CodeListPage/CodeLists/EditCodeList/EditCodeList.tsx (1)
1-1: LGTM! Consistent text resource handling implementation.The changes align with the standardized approach to text resource management across components, maintaining a clean and consistent implementation.
Also applies to: 23-23, 29-29, 35-35, 41-41, 70-72
frontend/libs/studio-components/src/components/StudioTextResourceInput/StudioTextResourceInput.tsx (1)
65-65: LGTM!The optional chaining operator is correctly implemented to safely handle the optional callback.
frontend/libs/studio-content-library/src/ContentLibrary/LibraryBody/pages/CodeListPage/CodeListPage.test.tsx (2)
187-220: LGTM! Comprehensive test coverage for text resource handling.The new test cases thoroughly verify:
- Text resource rendering in the create dialog
- Text resource updates with correct language handling
240-243: LGTM! Clean parameter update.The function modification improves reusability by accepting a generic
areaparameter instead of a specificaccordion.
...nd/libs/studio-components/src/components/StudioTextResourceInput/StudioTextResourceInput.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
frontend/libs/studio-content-library/src/ContentLibrary/LibraryBody/pages/CodeListPage/CodeListPage.tsx (2)
60-71: Optimize memoization dependencies.The
textResourcesForLanguagememoization should includelanguagein its dependency array since it's used within the memoized computation.const textResourcesForLanguage = useMemo( () => getTextResourcesForLanguage(language, textResources), - [textResources], + [textResources, language], );
112-112: Consider moving the language constant.The hardcoded language constant should be moved to a configuration file or constants file for better maintainability. This will make it easier to update when implementing the language selection feature referenced in issue #14572.
Consider creating a new file
constants.ts:export const DEFAULT_LANGUAGE = 'nb';Then import and use it in this file:
+import { DEFAULT_LANGUAGE } from './constants'; -const language: string = 'nb'; +const language: string = DEFAULT_LANGUAGE;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/libs/studio-content-library/src/ContentLibrary/LibraryBody/pages/CodeListPage/CodeListPage.tsx(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Build environment and run e2e test
- GitHub Check: CodeQL
- GitHub Check: Testing
🔇 Additional comments (2)
frontend/libs/studio-content-library/src/ContentLibrary/LibraryBody/pages/CodeListPage/CodeListPage.tsx (2)
1-15: LGTM! Well-organized imports.The new utility functions for text resource management are appropriately imported and organized.
89-107: LGTM! Consistent prop updates.The text resource handling props are consistently updated across child components, improving the overall data flow.
Description
This pull request adds text resources to the create code list dialog in the code list library. This includes moving some of the conversion functions up to the
CodeListPagelevel.I have added the
skip-manual-testinglabel since the this functionality is not yet in use.Related Issue(s)
Verification
Summary by CodeRabbit
New Features
Refactor
Tests