Skip to content
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

fix: Make label required #14826

Merged
merged 5 commits into from
Mar 3, 2025
Merged

fix: Make label required #14826

merged 5 commits into from
Mar 3, 2025

Conversation

TomasEng
Copy link
Contributor

@TomasEng TomasEng commented Feb 26, 2025

Description

This pull request makes the label field of the code list editor required, enfording the user to select a text resource for it.

Spiller.inn.2025-02-26.151912.mp4

Related Issue(s)

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)

Summary by CodeRabbit

  • New Features

    • Introduced new localized messages that inform users when no text resources are available in both Norwegian and English.
    • Enhanced text resource input components to clearly indicate required versus optional fields.
  • Tests

    • Added test cases to verify the proper display of empty state messages and the handling of required flags.
  • Refactor

    • Streamlined test data and property definitions for a more consistent user experience in text resource handling.

Copy link
Contributor

coderabbitai bot commented Feb 26, 2025

📝 Walkthrough

Walkthrough

This pull request introduces various updates to improve text resource handling. A new localization entry for empty text resources is added to the Norwegian language JSON file. Several components and their type definitions are extended to include a new optional or required property for text resource inputs. In addition, test cases, test data, story definitions, and hook functions are updated to incorporate an emptyTextResourceList property that delivers appropriate messages when text resources are not available. These changes span localization configurations, component implementations, tests, types, and hooks.

Changes

File(s) Change Summary
frontend/language/src/nb.json Added a new localization entry "code_list_editor.text_resource.empty_list": "Ingen tekstressurser er tilgjengelige" for empty text resources.
frontend/libs/…/StudioCodelistEditor.test.tsx Introduced and consolidated test cases using propsWithTextResources to verify the correct display of unset options in the code list editor.
frontend/libs/…/StudioCodelistEditorRow/StudioCodeListEditorRow.tsx, frontend/libs/…/StudioTextResourceInput/StudioTextResourceInput.tsx, frontend/libs/…/StudioTextResourceInput/types/TextResourceInputTexts.ts Updated component APIs and type definitions by adding a required property, marking noTextResourceOptionLabel optional, and integrating an emptyTextResourceList field.
frontend/libs/…/StudioCodelistEditor/test-data/texts.ts, frontend/libs/…/StudioInputTable/test-data/testTableData.ts, frontend/libs/…/StudioTextResourceInput/StudioTextResourceInput.stories.tsx, frontend/libs/…/StudioTextResourceInput/StudioTextResourceInput.test.tsx Added the emptyTextResourceList property with localized messages to test data and story objects to handle scenarios with no available text resources.
frontend/libs/…/ContentLibrary/LibraryBody/pages/CodeListPage/hooks/useCodeListEditorTexts.ts, frontend/packages/ux-editor/src/…/useOptionListEditorTexts.ts Enhanced hook functions by adding an emptyTextResourceList property derived from translation keys in their returned objects.

Possibly related PRs

  • fix: Consistent terms in code list editor #14780: The changes in the main PR, which add a new entry for Norwegian language localization in the JSON file, are related to the modifications in the retrieved PR that also involve updates to the same JSON file for localization, specifically focusing on terminology consistency.
  • fix: Text resource unsetting issues #14568: The changes in the main PR, which involve adding a new entry for Norwegian language localization regarding empty text resources, are related to the modifications in the retrieved PR that address the handling of unset text resources in the StudioTextResourcePicker, as both involve the management and display of text resource states.
  • fix: Support unsetting the text resource in the text resource picker #14496: The changes in the main PR, which add a new entry for empty text resources in the Norwegian localization JSON, are related to the retrieved PR that modifies the same JSON file by removing an existing entry for empty text resources and adding a different label for unset options. Both PRs directly affect the frontend/language/src/nb.json file and deal with text resource messages.

Suggested labels

area/content-library, skip-documentation

Suggested reviewers

  • github-actions
  • ErlingHauan

Warning

There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

frontend/libs/studio-components/src/components/StudioTextResourceInput/StudioTextResourceInput.test.tsx

Oops! Something went wrong! :(

ESLint: 8.57.1

ESLint couldn't find the plugin "eslint-plugin-storybook".

(The package "eslint-plugin-storybook" was not found when loaded as a Node module from the directory "/frontend/libs/studio-components".)

It's likely that the plugin isn't installed correctly. Try reinstalling by running the following:

npm install eslint-plugin-storybook@latest --save-dev

The plugin "eslint-plugin-storybook" was referenced from the config file in "frontend/libs/studio-components/.eslintrc.js".

If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team.

frontend/libs/studio-components/src/components/StudioTextResourceInput/StudioTextResourceInput.tsx

Oops! Something went wrong! :(

ESLint: 8.57.1

ESLint couldn't find the plugin "eslint-plugin-storybook".

(The package "eslint-plugin-storybook" was not found when loaded as a Node module from the directory "/frontend/libs/studio-components".)

It's likely that the plugin isn't installed correctly. Try reinstalling by running the following:

npm install eslint-plugin-storybook@latest --save-dev

The plugin "eslint-plugin-storybook" was referenced from the config file in "frontend/libs/studio-components/.eslintrc.js".

If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team.


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9f7002f and 26503dd.

📒 Files selected for processing (3)
  • frontend/language/src/nb.json (1 hunks)
  • frontend/libs/studio-components/src/components/StudioTextResourceInput/StudioTextResourceInput.test.tsx (1 hunks)
  • frontend/libs/studio-components/src/components/StudioTextResourceInput/StudioTextResourceInput.tsx (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
  • frontend/libs/studio-components/src/components/StudioTextResourceInput/StudioTextResourceInput.tsx
  • frontend/libs/studio-components/src/components/StudioTextResourceInput/StudioTextResourceInput.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

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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@github-actions 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 Feb 26, 2025
Copy link

codecov bot commented Feb 26, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.79%. Comparing base (8fec646) to head (26503dd).
Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #14826   +/-   ##
=======================================
  Coverage   95.79%   95.79%           
=======================================
  Files        1924     1924           
  Lines       25107    25107           
  Branches     2869     2869           
=======================================
  Hits        24050    24050           
  Misses        797      797           
  Partials      260      260           

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

# Conflicts:
#	frontend/libs/studio-components/src/components/StudioCodelistEditor/StudioCodeListEditor.test.tsx
@TomasEng TomasEng changed the title Make label required fix: Make label required Feb 27, 2025
@TomasEng TomasEng marked this pull request as ready for review February 27, 2025 09:55
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (2)
frontend/packages/ux-editor/src/components/config/editModal/EditOptions/OptionTabs/hooks/useOptionListEditorTexts.ts (1)

47-50: New property added alongside existing similar property

The addition of emptyTextResourceList provides a dedicated property for empty text resource messaging. However, I notice it uses the same translation key as the existing emptyResourceList property on the next line. Consider if maintaining both properties with identical values might cause confusion for future maintainers.

If both properties are intentionally maintained for separate use cases, consider adding a code comment explaining the distinction between them to improve code maintainability.

frontend/language/src/nb.json (1)

175-175: Potential Typo in Localization Entry

The new key "code_list_editor.text_resource.empty_list" has been added with the value "Ingen tekstressurser er tigjengelige". It appears that the word "tigjengelige" may be a typo and should likely be corrected to "tilgjengelige" to ensure proper Norwegian spelling.

-  "code_list_editor.text_resource.empty_list": "Ingen tekstressurser er tigjengelige",
+  "code_list_editor.text_resource.empty_list": "Ingen tekstressurser er tilgjengelige",
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 085d2c5 and 5bdcb8b.

📒 Files selected for processing (11)
  • frontend/language/src/nb.json (1 hunks)
  • frontend/libs/studio-components/src/components/StudioCodelistEditor/StudioCodeListEditor.test.tsx (2 hunks)
  • frontend/libs/studio-components/src/components/StudioCodelistEditor/StudioCodeListEditorRow/StudioCodeListEditorRow.tsx (6 hunks)
  • frontend/libs/studio-components/src/components/StudioCodelistEditor/test-data/texts.ts (1 hunks)
  • frontend/libs/studio-components/src/components/StudioInputTable/test-data/testTableData.ts (1 hunks)
  • frontend/libs/studio-components/src/components/StudioTextResourceInput/StudioTextResourceInput.stories.tsx (1 hunks)
  • frontend/libs/studio-components/src/components/StudioTextResourceInput/StudioTextResourceInput.test.tsx (1 hunks)
  • frontend/libs/studio-components/src/components/StudioTextResourceInput/StudioTextResourceInput.tsx (3 hunks)
  • frontend/libs/studio-components/src/components/StudioTextResourceInput/types/TextResourceInputTexts.ts (1 hunks)
  • frontend/libs/studio-content-library/src/ContentLibrary/LibraryBody/pages/CodeListPage/hooks/useCodeListEditorTexts.ts (1 hunks)
  • frontend/packages/ux-editor/src/components/config/editModal/EditOptions/OptionTabs/hooks/useOptionListEditorTexts.ts (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
frontend/libs/studio-components/src/components/StudioTextResourceInput/StudioTextResourceInput.tsx (1)
Learnt from: TomasEng
PR: Altinn/altinn-studio#14646
File: frontend/libs/studio-components/src/components/StudioTextResourceInput/StudioTextResourceInput.tsx:30-30
Timestamp: 2025-02-13T06:29:43.924Z
Learning: TypeScript's `strictNullChecks` mode helps catch potential null/undefined errors at compile time, making it easier to identify issues with optional properties and callbacks.
🔇 Additional comments (18)
frontend/libs/studio-components/src/components/StudioTextResourceInput/StudioTextResourceInput.stories.tsx (1)

39-46: Added property for empty text resource list message in story args

The addition of the emptyTextResourceList property in the texts object properly aligns with the PR objectives, providing a user-friendly message when no text resources are available. This change ensures consistent messaging throughout the application.

frontend/libs/studio-content-library/src/ContentLibrary/LibraryBody/pages/CodeListPage/hooks/useCodeListEditorTexts.ts (1)

48-48: Good addition for handling empty text resource lists!

Adding the emptyTextResourceList property improves user experience by providing a message when no text resources are available. The implementation correctly uses the translation key structure.

frontend/libs/studio-components/src/components/StudioCodelistEditor/test-data/texts.ts (1)

49-49: Test data updated to match the new interface requirements.

The addition of emptyTextResourceList to the test data ensures consistency with the updated component interface. This will help maintain test coverage for scenarios with empty text resource lists.

frontend/libs/studio-components/src/components/StudioTextResourceInput/types/TextResourceInputTexts.ts (1)

7-8: Updated type definition with appropriate optionality.

The interface changes align well with the PR objective:

  1. Adding the optional emptyTextResourceList property provides a way to display messages when no text resources are available
  2. Making noTextResourceOptionLabel optional allows for more flexibility in component usage

These changes improve the component's adaptability while maintaining backward compatibility.

frontend/libs/studio-components/src/components/StudioTextResourceInput/StudioTextResourceInput.test.tsx (1)

20-20: Test data updated with Norwegian localization for empty text resources.

Good addition of the emptyTextResourceList property with the Norwegian text "Ingen tekstressurser er tilgjengelige". This ensures that tests properly verify the component's behavior when dealing with empty text resource lists.

frontend/libs/studio-components/src/components/StudioTextResourceInput/StudioTextResourceInput.tsx (3)

31-31: Added required property to support validation

This addition of the optional required property to the TextResourceInputPropsBase type allows the component to support input validation, aligning with the PR objective to make labels required.


107-107: Good addition of required property to destructured props

Properly destructuring the required property from the props object to make it available within the InputBox component.


138-138: Correctly passing required property to StudioTextResourcePicker

Passing the required property to the StudioTextResourcePicker enables validation at the picker level, ensuring it can properly communicate the required state to users.

frontend/libs/studio-components/src/components/StudioInputTable/test-data/testTableData.ts (1)

38-38: Added feedback message for empty text resources

Added a clear user feedback message for scenarios when no text resources are available, improving the user experience.

frontend/libs/studio-components/src/components/StudioCodelistEditor/StudioCodeListEditorRow/StudioCodeListEditorRow.tsx (6)

83-83: Correctly marking label field as required

Setting the label field as required aligns with the PR objective to make labels mandatory, ensuring users provide this important information.


93-93: Appropriately marking description as optional

Correctly setting the description field as optional, maintaining flexibility for users while enforcing requirements only where necessary.


103-103: Appropriately marking help text as optional

Correctly setting the help text field as optional, consistent with the approach used for the description field.


219-219: Added required property to TextResourceIdCellProps type

Adding the required property to the props type ensures type safety and proper documentation of the component API.


238-238: Properly receiving required prop in TextResourceSelectorCell

Correctly destructuring the required prop from the component properties.


249-249: Properly passing required prop to child component

Correctly passing the required property down to the StudioInputTable.Cell.TextResource component, ensuring the validation requirement is propagated through the component hierarchy.

frontend/libs/studio-components/src/components/StudioCodelistEditor/StudioCodeListEditor.test.tsx (3)

44-47: Good test setup for text resources

Created a reusable props object with text resources for testing, improving test organization and maintainability.


102-111: Test ensuring labels cannot be unset

Great test that verifies the required behavior for labels - confirming that the "unset" option is not available for required fields.


113-125: Comprehensive testing of optional fields

Excellent parameterized test that verifies both description and help text fields (which are optional) correctly display the "unset" option. This approach reduces test duplication and ensures both optional properties are properly tested.

@wrt95 wrt95 self-assigned this Feb 27, 2025
Copy link
Contributor

@wrt95 wrt95 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved, but one small comment on language typo 😄

@wrt95 wrt95 assigned TomasEng and unassigned wrt95 Feb 27, 2025
Co-authored-by: William Thorenfeldt <48119543+wrt95@users.noreply.github.com>
Copy link
Contributor

@Konrad-Simso Konrad-Simso left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested ok, good work!

@Konrad-Simso Konrad-Simso merged commit a9d16f7 into main Mar 3, 2025
11 checks passed
@Konrad-Simso Konrad-Simso deleted the make-label-required branch March 3, 2025 12:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ui-editor Area: Related to the designer tool for assembling app UI in Altinn Studio. frontend solution/studio/designer Issues related to the Altinn Studio Designer solution. team/studio-domain1
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

3 participants