Skip to content

Expressions: Returning null when component is hidden #635

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

Merged
merged 18 commits into from
Nov 15, 2022

Conversation

olemartinorg
Copy link
Contributor

@olemartinorg olemartinorg commented Nov 8, 2022

Description

In the component lookup function, the returned value should be null if the component itself is hidden.

The reason this PR is so large for such a minor feature, is mostly because of the way expressions were resolved - they did not have all layouts (in a layout set) available at all times, so some code had to be refactored to make sure of that. Also, this leads to some complexity when factoring in that hiding an entire layout should hide all components within it - and hiding such a component by hiding the entire layout should make the component lookup return null even if the component has a data model value. The added cypress test reflects one of the more complex use-cases this solves.

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)
  • All tests run green

Documentation

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

Ole Martin Handeland added 11 commits November 8, 2022 13:56
…iddenFields first (just like in real code)
# Conflicts:
#	src/altinn-app-frontend/src/features/expressions/shared.test.ts
…k up the component ID in the collection of root nodes - merely using the currentLayout as a suggestion on where to look first.
…ges. It all came to a halt when I found that even resolvedLayoutsFromState() only resolves from the context of the current layout, so a bigger change is needed to make this work.
…caused an infinite loop in useExpressions.test.tsx
… makes multi-page component lookups work properly.
… sure to re-run expressions after that to catch potential dependencies.
… In the test-code, the hidden property was placed on the wrong level in the tree.
@github-actions github-actions bot added the area/test related to automated and functional testing label Nov 11, 2022
Ole Martin Handeland added 5 commits November 11, 2022 16:26
…rcepted by the last intercept call when it reloaded
# Conflicts:
#	test/cypress/e2e/integration/app-frontend/group.js
#	test/cypress/e2e/support/app-frontend.js
#	test/cypress/e2e/support/index.d.ts
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

82.0% 82.0% Coverage
0.0% 0.0% Duplication

@olemartinorg olemartinorg marked this pull request as ready for review November 15, 2022 08:41
@olemartinorg olemartinorg added the kind/product-feature Pull requests containing new features label Nov 15, 2022
Copy link
Member

@bjosttveit bjosttveit left a comment

Choose a reason for hiding this comment

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

LGTM 🥇

@olemartinorg olemartinorg merged commit dd5b821 into main Nov 15, 2022
@olemartinorg olemartinorg deleted the feat/expr-component-hidden branch November 15, 2022 10:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/test related to automated and functional testing kind/product-feature Pull requests containing new features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Expressions: New not function, extend logic behind component
2 participants