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: Error when changing Group heading level #13692

Merged
merged 9 commits into from
Oct 9, 2024

Conversation

ErlingHauan
Copy link
Contributor

@ErlingHauan ErlingHauan commented Oct 3, 2024

Description

This PR fixes the error when changing headingLevel in a Group component.

  • Changed type of property from string to number in Group.schema.v1.json
  • Changed the checks in component.ts. If the type is not given for a component propery in the schema, the code will now check the type of the first element of the enum, and either render EditStringValue or EditNumberValue based on the element's type.
  • EditNumberValue now accepts an enum value, similar to EditStringValue. If an enum is present, it will render a native select instead of an input field.
  • Added more tests to EditNumberValue and EditStringValue.
  • Added missing text resources for the groupingIndicator property enum.

Note that this fix will no longer work the next time the JSON schema component script is run, because it will replace "type": "number" with "type": "string". The PR that fixes the script is here. Made this a separate PR so that we can get the fix into prod as soon as possible.

Related Issue(s)

Videos

Before

heading-level-before.mp4

After

heading-level-after.mp4

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)

@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. labels Oct 3, 2024
@ErlingHauan ErlingHauan changed the title Fix: Error when changing Group heading level fix: Error when changing Group heading level Oct 3, 2024
Copy link

codecov bot commented Oct 3, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.41%. Comparing base (939464c) to head (5fe11bc).
Report is 17 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #13692   +/-   ##
=======================================
  Coverage   94.41%   94.41%           
=======================================
  Files        1574     1574           
  Lines       21276    21280    +4     
  Branches     2541     2543    +2     
=======================================
+ Hits        20088    20092    +4     
  Misses        943      943           
  Partials      245      245           

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

@ErlingHauan ErlingHauan marked this pull request as ready for review October 3, 2024 10:29
@ErlingHauan ErlingHauan linked an issue Oct 4, 2024 that may be closed by this pull request
@mlqn mlqn self-assigned this Oct 8, 2024
Copy link
Contributor

@mlqn mlqn left a comment

Choose a reason for hiding this comment

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

Nice work! 💯

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.

Tested OK in dev 😄

@wrt95 wrt95 merged commit 6a27c3a into main Oct 9, 2024
17 checks passed
@wrt95 wrt95 deleted the fix/error-when-changing-group-heading-level branch October 9, 2024 11:40
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-domain2
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants