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

feat: 13526 support component specific overrides in summary2 #13734

Merged
merged 29 commits into from
Oct 17, 2024

Conversation

JamalAlabdullah
Copy link
Contributor

@JamalAlabdullah JamalAlabdullah commented Oct 7, 2024

Description

These changes are behind the featureflag summary2

Changes:

  • Render a dropdown menu with three options: (string, list, not set) when the summary is for a checkbox or multiselect component.
  • Render a checkbox (isCompact) when the summary is for a component that is inside a group component.
  • Add tests.

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)

Documentation

  • User documentation is updated with a separate linked PR in altinn-studio-docs. (if applicable)

@JamalAlabdullah JamalAlabdullah linked an issue Oct 7, 2024 that may be closed by this pull request
3 tasks
@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 Oct 7, 2024
@JamalAlabdullah JamalAlabdullah changed the title Feat13526 support component specific overrides in summary2 feat:13526 support component specific overrides in summary2 Oct 7, 2024
@JamalAlabdullah JamalAlabdullah changed the title feat:13526 support component specific overrides in summary2 feat: 13526 support component specific overrides in summary2 Oct 7, 2024
Copy link

codecov bot commented Oct 7, 2024

Codecov Report

Attention: Patch coverage is 97.56098% with 1 line in your changes missing coverage. Please review.

Project coverage is 95.27%. Comparing base (2f17f0a) to head (150ca0d).
Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
...ontent/Summary2/Override/Summary2OverrideEntry.tsx 92.85% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main   #13734   +/-   ##
=======================================
  Coverage   95.27%   95.27%           
=======================================
  Files        1635     1638    +3     
  Lines       21828    21867   +39     
  Branches     2567     2573    +6     
=======================================
+ Hits        20796    20834   +38     
  Misses        787      787           
- Partials      245      246    +1     

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

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.

Great job Jamal! 👏
I've added some comments 😄

@JamalAlabdullah JamalAlabdullah requested a review from wrt95 October 11, 2024 06:55
@JamalAlabdullah JamalAlabdullah requested a review from wrt95 October 11, 2024 12:49
@JamalAlabdullah JamalAlabdullah requested a review from wrt95 October 15, 2024 07:13
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.

Great job @JamalAlabdullah! 👏 🥳

Copy link
Member

@Jondyr Jondyr left a comment

Choose a reason for hiding this comment

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

Tested and works without any issues 🎉

But i have one question; for group overrides, the documentation states:

In addition, a few components support component-specific overrides:
isCompact [...] Option to display a compact version of groups.

ref. https://docs.altinn.studio/altinn-studio/reference/ux/components/summary2/

Which i interpret as allowing an override on a whole group container, but the current solution seems to enable this field for components inside a group.
I'm wondering if this is a confusion about containers/components, or if this is the intended behaviour?
The isCompact override does not seem to have any effect on the summary2 component in the preview, from my testing.

@framitdavid
Copy link
Collaborator

Tested and works without any issues 🎉

But i have one question; for group overrides, the documentation states:

In addition, a few components support component-specific overrides:
isCompact [...] Option to display a compact version of groups.

ref. https://docs.altinn.studio/altinn-studio/reference/ux/components/summary2/

Which i interpret as allowing an override on a whole group container, but the current solution seems to enable this field for components inside a group. I'm wondering if this is a confusion about containers/components, or if this is the intended behaviour? The override does not seem to have any effect on the summary2 component in the preview, from my testing.

Nice catch, @Jondyr! That’s correct, the isCompact should be on the group component itself.

But my understanding, team apps will be supporting isCompact as a global config later (not sure when), but for now it’s only the group component as a container. 😀

@JamalAlabdullah
Copy link
Contributor Author

Tested and works without any issues 🎉
But i have one question; for group overrides, the documentation states:

In addition, a few components support component-specific overrides:
isCompact [...] Option to display a compact version of groups.

ref. https://docs.altinn.studio/altinn-studio/reference/ux/components/summary2/
Which i interpret as allowing an override on a whole group container, but the current solution seems to enable this field for components inside a group. I'm wondering if this is a confusion about containers/components, or if this is the intended behaviour? The override does not seem to have any effect on the summary2 component in the preview, from my testing.

Nice catch, @Jondyr! That’s correct, the isCompact should be on the group component itself.

But my understanding, team apps will be supporting isCompact as a global config later (not sure when), but for now it’s only the group component as a container. 😀

@framitdavid @Jondyr I have changed the logic to check against the group component itself. 🙂

Copy link
Member

@Jondyr Jondyr left a comment

Choose a reason for hiding this comment

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

Nice 👍 Tested and works well ✅

I have a question about a code change though:

Copy link
Member

@Jondyr Jondyr left a comment

Choose a reason for hiding this comment

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

Great work!

@wrt95 wrt95 merged commit 6c1f174 into main Oct 17, 2024
10 checks passed
@wrt95 wrt95 deleted the feat13526-support-component-specific-overrides-in-summary2 branch October 17, 2024 12:52
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support component specific overrides in Summary2
4 participants