fix: update app title in all available languages #14546
Conversation
📝 WalkthroughWalkthroughThe pull request introduces significant changes to the application's settings modal, specifically in the About Tab and related components. The modifications focus on refactoring how application metadata and service names are handled, with an emphasis on language-specific configurations. The changes involve updating test files, component implementations, and localization strings to support more flexible and language-aware service name management. 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
standeren
added
kind/bug
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #14546 +/- ##
==========================================
- Coverage 95.71% 95.70% -0.02%
==========================================
Files 1904 1903 -1
Lines 24801 24805 +4
Branches 2840 2841 +1
==========================================
+ Hits 23738 23739 +1
- Misses 802 805 +3
Partials 261 261 ☔ View full report in Codecov by Sentry. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (7)
frontend/app-development/layout/PageHeader/SubHeader/SettingsModalButton/SettingsModal/components/Tabs/AboutTab/AboutTab.test.tsx (4)
Line range hint
54-69: Consider adding specific error scenario tests.While the current parametrized test is good, consider adding tests for specific error scenarios (e.g., network errors, validation errors) to ensure proper error handling for different failure modes.
80-89: Consider adding input validation tests.While the basic input functionality is tested, consider adding tests for:
- Maximum length validation
- Special character handling
- Empty string validation
92-148: Consider adding edge cases for language handling.The language-specific tests are well-structured, but consider adding tests for:
- Handling of RTL languages
- Unicode character support in titles
- Maximum length validation per language
198-220: Consider enhancing helper function robustness.The helper functions could benefit from:
- Timeout handling for waitForElementToBeRemoved
- Error handling for failed renders
- Type validation for input parameters
Example implementation for timeout handling:
const resolveAndWaitForSpinnerToDisappear = async ( queries: Partial<ServicesContextProps> = {}, previewContextProps: Partial<PreviewContextProps> = {}, ) => { const getTextLanguages = jest.fn().mockImplementation(() => Promise.resolve([DEFAULT_LANGUAGE])); const getRepoMetadata = jest.fn().mockImplementation(() => Promise.resolve(mockRepository1)); const getAppMetadata = jest.fn().mockImplementation(() => Promise.resolve(mockAppMetadata)); renderAboutTab( { getTextLanguages, getRepoMetadata, getAppMetadata, ...queries }, { ...previewContextProps }, ); - await waitForElementToBeRemoved(() => - screen.queryByTitle(textMock('settings_modal.loading_content')), - ); + await waitForElementToBeRemoved( + () => screen.queryByTitle(textMock('settings_modal.loading_content')), + { timeout: 5000 } + ).catch(() => { + throw new Error('Spinner did not disappear within timeout period'); + }); };frontend/app-development/layout/PageHeader/SubHeader/SettingsModalButton/SettingsModal/components/Tabs/AboutTab/AboutTab.tsx (2)
45-49: Reload after successful title update.
doReloadPreview()is triggered before callingupdateAppTitle. IfupdateAppTitleis an async mutation, consider waiting for it to complete before reloading the preview for a more predictable user experience.
60-60: Combine error messaging if needed.Currently, each error is displayed separately. You could unify them in a single container or return them in a mapped list if avoiding repeated
<ErrorMessage>calls is desired.frontend/app-development/layout/PageHeader/SubHeader/SettingsModalButton/SettingsModal/components/Tabs/AboutTab/InputFields/InputFields.test.tsx (1)
66-66: Consider adding tests for read-only recommended languages without resource files.
Currently, the tests confirm behavior for an existing app language (e.g.,nb). To strengthen coverage, add a scenario where a recommended language is absent fromappLangCodesto verify that the input is read-only and validation is skipped.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
frontend/app-development/layout/PageHeader/SubHeader/SettingsModalButton/SettingsModal/SettingsModal.test.tsx(1 hunks)frontend/app-development/layout/PageHeader/SubHeader/SettingsModalButton/SettingsModal/components/Tabs/AboutTab/AboutTab.test.tsx(5 hunks)frontend/app-development/layout/PageHeader/SubHeader/SettingsModalButton/SettingsModal/components/Tabs/AboutTab/AboutTab.tsx(3 hunks)frontend/app-development/layout/PageHeader/SubHeader/SettingsModalButton/SettingsModal/components/Tabs/AboutTab/InputFields/InputFields.module.css(1 hunks)frontend/app-development/layout/PageHeader/SubHeader/SettingsModalButton/SettingsModal/components/Tabs/AboutTab/InputFields/InputFields.test.tsx(2 hunks)frontend/app-development/layout/PageHeader/SubHeader/SettingsModalButton/SettingsModal/components/Tabs/AboutTab/InputFields/InputFields.tsx(1 hunks)frontend/app-development/layout/PageHeader/SubHeader/SettingsModalButton/SettingsModal/mocks/appConfigMock.ts(0 hunks)frontend/language/src/nb.json(1 hunks)frontend/packages/shared/src/mocks/mocks.ts(1 hunks)
💤 Files with no reviewable changes (1)
- frontend/app-development/layout/PageHeader/SubHeader/SettingsModalButton/SettingsModal/mocks/appConfigMock.ts
✅ Files skipped from review due to trivial changes (1)
- frontend/app-development/layout/PageHeader/SubHeader/SettingsModalButton/SettingsModal/components/Tabs/AboutTab/InputFields/InputFields.module.css
🧰 Additional context used
🪛 Biome (1.9.4)
frontend/app-development/layout/PageHeader/SubHeader/SettingsModalButton/SettingsModal/components/Tabs/AboutTab/InputFields/InputFields.tsx
[error] 100-101: Don't use 'Boolean' as a type.
Use lowercase primitives for consistency.
Safe fix: Use 'boolean' instead
(lint/complexity/noBannedTypes)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Build environment and run e2e test
- GitHub Check: Testing
🔇 Additional comments (36)
frontend/app-development/layout/PageHeader/SubHeader/SettingsModalButton/SettingsModal/components/Tabs/AboutTab/AboutTab.test.tsx (3)
17-20: Well-structured language constants!Good practice using constants for language codes instead of magic strings. This improves maintainability and reduces the risk of typos.
Line range hint
33-38: Good test for initial loading state!The test properly verifies the loading indicator's presence, which is crucial for user experience.
39-52: Well-structured data fetching tests!Good separation of concerns with individual tests for each API call. This makes it easier to identify which fetch operation failed when tests break.
frontend/app-development/layout/PageHeader/SubHeader/SettingsModalButton/SettingsModal/SettingsModal.test.tsx (2)
8-8: LGTM! Good addition of type import.The explicit import of QueryClient type improves type safety.
Line range hint
1-150: Verify test coverage for new language-related functionality.The test file has good coverage of modal behavior and tab navigation, but seems to be missing tests for the new language-related functionality mentioned in the PR objectives (available languages display, recommended languages, app name updates).
Let's verify the test coverage:
frontend/app-development/layout/PageHeader/SubHeader/SettingsModalButton/SettingsModal/components/Tabs/AboutTab/AboutTab.tsx (6)
15-17: Ensure typed imports remain consistent.These added imports look correctly typed. Just make sure all references for
ServiceNames,useUpdateAppTitle, anduseLanguagesQueryremain consistent throughout the codebase.
28-31: Verify fallback for missing language data.When
appLangCodesDatais undefined or empty, ensure that the component still renders gracefully. Consider adding defensive checks before consumingappLangCodesData.
43-43: Confirm stable usage of the update hook.Invoking
useUpdateAppTitle(applicationMetadataData)at the top level is fine. Ensure that no re-render side effects inadvertently occur ifapplicationMetadataDatais re-fetched and remains unchanged.
53-53: No issues with the switch statement.This status-based render logic is clear. The fallback steps for pending, success, and error states are correctly implemented.
69-72: Handle missing or partial titles.When constructing
appTitles, ensure you handle any missing properties inapplicationMetadataData.titleorappLangCodesData. Consider fallback checks so the UI doesn’t break on partially-loaded data.
75-80: Ensure repository data is loaded.If
repositoryDatais still loading or unavailable, adding optional chaining or fallback text forrepositoryData.namecan prevent runtime issues.frontend/packages/shared/src/mocks/mocks.ts (1)
171-173: Added title field looks good.Adding
titletoapplicationMetadatais straightforward. If you plan to add more localized keys, be sure to handle them consistently in the consuming components.frontend/language/src/nb.json (2)
1071-1071: New translation key for missing translation file.The new key
"settings_modal.about_tab_app_title_no_translation_file"is well-structured and consistent with naming conventions.
1077-1077: Updated app name advice text.The updated description encouraging Bokmål, Nynorsk, and English is succinct and user-friendly.
frontend/app-development/layout/PageHeader/SubHeader/SettingsModalButton/SettingsModal/components/Tabs/AboutTab/InputFields/InputFields.tsx (15)
1-2: Good import structure.
The updated imports from React and CSS modules are properly organized.
5-10: Successful transition to Studio components.
ReplacingTextfieldwithStudioTextfieldand related components aligns well with the standardized component library.
12-14: Ensure consistency with prop usage.
TheserviceNamestype correctly allows forundefined. However, verify that consumer components, especially inEditServiceNameForLanguage, also handleundefinedsafely.
16-20: Well-defined recommended language flags.
Using an enum for recognized flags helps maintain clarity regarding which languages are fully recommended or officially supported.
22-26: Props definition aligns with new approach.
The renamed props focus on language-based service names, removing dependency onappConfig. This fosters a more modular structure.
29-47: Clear separation of repository name from service names.
Using a read-only field to display the repo name is a good UX measure. The overall layout in this component remains clean and intuitive.
49-50: Omitting 'repositoryName' in the sub-props is a logical refinement.
This ensuresEditServiceNamesonly deals with language-specific service name handling.
51-75: Straightforward iteration over service name languages.
Iterating over all keys inserviceNamesis a clean way to handle the service name fields for each language.
84-92: Effective local error state management.
UsinguseStatefor storing error messages is appropriate for onBlur validation logic.
93-94: Good presence check for translation file.
Conditionally togglingreadOnlybased on the presence of the language inappLangCodesensures that the user doesn’t mistakenly edit a language that doesn't have a dedicated resource file.
95-97: onBlur validation is straightforward.
Fine-grained form validation on blur fosters a good user experience and simpler code.
109-111: Concise external description assignment.
It’s good practice to gather the computed description in a local variable for clarity.
113-123: Appropriate usage of StudioIconTextfield.
Providing an icon for each language and an optional readonly property effectively combines clarity with controlled editing.
127-130: Type extraction for flag icons is adequately scoped.
DefiningFlagIconPropswithin the same file keeps implementation cohesive.
131-132: Fallback for unsupported flags is smooth.
Returningnullwhen a language flag isn’t available ensures minimal visual interruptions.frontend/app-development/layout/PageHeader/SubHeader/SettingsModalButton/SettingsModal/components/Tabs/AboutTab/InputFields/InputFields.test.tsx (7)
3-3: Accurate import usage of new types.
ImportingInputFieldsPropsandServiceNamesaffirms alignment with the updated props.
9-15: Appropriate mock data setup.
Using typed mock data ensures test coverage for the newly introduced props likeappLangCodesandserviceNames.
17-21: Well-defined defaultProps.
Gathering default props in a dedicated object fosters test readability and consistency across multiple test cases.
27-31: Read-only repository name assertion is correctly tested.
Confirming that the input is both read-only and accurately containsrepositoryNameis an important validation step.
35-44: Thorough testing of language-specific service name input.
Ensuring the correct value is displayed and can be updated verifies the core business logic of localized service name handling.
49-57: Validation success scenario is clearly covered.
Verifying thatonSaveis called upon losing focus confirms that valid changes are indeed committed.
66-66: Validation failure scenario is clearly covered.
Testing thatonSaveisn’t called when the service name is invalid ensures robust validation.
...eader/SettingsModalButton/SettingsModal/components/Tabs/AboutTab/InputFields/InputFields.tsx
Outdated
Show resolved
Hide resolved
...eader/SettingsModalButton/SettingsModal/components/Tabs/AboutTab/InputFields/InputFields.tsx
Outdated
Show resolved
Hide resolved
standeren
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
frontend/app-development/layout/PageHeader/SubHeader/SettingsModalButton/SettingsModal/components/Tabs/AboutTab/InputFields/InputFields.tsx (2)
94-101: Separate validation logic from side effects.The validation function currently handles both validation and error state management. Consider separating these concerns for better maintainability and testability.
-const validateServiceName = (newServiceName: string): boolean => { +const isServiceNameValid = (newServiceName: string): boolean => { + return newServiceName.length > 0; +}; + +const validateServiceName = (newServiceName: string): boolean => { + const isValid = isServiceNameValid(newServiceName); if (newServiceName.length <= 0) { setServiceNameError(t('settings_modal.about_tab_name_error')); return false; } setServiceNameError(''); return true; };
89-92: Consider validating on change for better UX.The current implementation only validates on blur, which might delay error feedback to users. Consider adding onChange validation for a more responsive user experience.
+const handleOnChange = (event: ChangeEvent<HTMLInputElement>) => { + const isValid = isServiceNameValid(event.target.value); + if (!isValid) { + setServiceNameError(t('settings_modal.about_tab_name_error')); + } else { + setServiceNameError(''); + } +}; const handleOnBlur = (event: ChangeEvent<HTMLInputElement>) => { const isValid = validateServiceName(event.target.value); if (isValid) onSave(event.target.value, language); };Then update the StudioIconTextfield:
<StudioIconTextfield size='small' label={t(`language.${language}`)} description={description} error={serviceNameError} value={serviceName} onBlur={handleOnBlur} + onChange={handleOnChange} readOnly={!appHasLanguageTranslation} className={classes.serviceName} />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/app-development/layout/PageHeader/SubHeader/SettingsModalButton/SettingsModal/components/Tabs/AboutTab/InputFields/InputFields.tsx(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Build environment and run e2e test
- GitHub Check: Testing
🔇 Additional comments (3)
frontend/app-development/layout/PageHeader/SubHeader/SettingsModalButton/SettingsModal/components/Tabs/AboutTab/InputFields/InputFields.tsx (3)
12-20: LGTM! Well-structured type definitions.The generic type definitions are well-designed, with proper handling of undefined values and clear prop interfaces.
23-41: LGTM! Clean component structure.The component has a clear separation of concerns and proper prop handling. The read-only repository name field is well-implemented.
43-69: LGTM! Well-implemented service names editor.The component effectively manages multiple language entries with proper React patterns and UI components.
Description
The updated view in
Aboutin the settings modal shows all available languages the app has, which is deduced from the language resource files in the app. The view does also include the recommended languages, nn, nb and en, but as readonly if the app does not have resource files for these. The same accounts for other titles existing in in appmetadata that does not have resource files for the languages.When updating a app name in a language, the app name is updated both in resource file and in applicationmetadata, without overriding the existing title in applicationmetadata. Also, the
.configfile is not written to repo. This controller can be deleted.Related Issue(s)
appNamedoesn't updatetitleinapplicationMetaData#12917Verification
Summary by CodeRabbit
Summary by CodeRabbit
Refactor
New Features
Documentation
Bug Fixes
Style