Skip to content

Expressions #540

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 148 commits into from
Oct 26, 2022
Merged

Expressions #540

merged 148 commits into from
Oct 26, 2022

Conversation

olemartinorg
Copy link
Contributor

@olemartinorg olemartinorg commented Oct 13, 2022

Description

This PR adds support for dynamic expressions, as used in the following component properties:

  • required
  • readOnly
  • hidden (new!)
  • edit.addButton (for repeating groups)
  • edit.saveButton (for repeating groups)
  • edit.deleteButton (for repeating groups)
  • edit.saveAndNextButton (for repeating groups)

Along with the following properties for layouts/pages:

  • hidden (new!) This makes it an alternative to tracks.

It includes the following expression functions for now:

  • equals / notEquals
  • greaterThan / greaterThanEq / lessThan / lessThanEq
  • concat
  • and / or
  • if (beta!) - else branch is optionally passed in with parameters
  • instanceContext
  • frontendSettings
  • component (resolves upwards inside repeating groups)
  • dataModel (resolves its position inside repeating groups without the need for position placeholders)

Try this out using the test app: https://dev.altinn.studio/repos/olemartinorg/layout-expressions

Because of the size of this PR, we'll do a review meeting instead of performing code review inside github. Marking this as draft until that meeting is over.

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

Ole Martin Handeland added 30 commits July 29, 2022 19:26
…s (at least when used as constants, not when in argument expressions)
… hackaton/dynamics branch. Fixing some bugs in them, adjusting them to new typings, writing unit tests.
…tNode object on top (to be able to use closest() to find nodes on the top level), I found that it would be better to keep control inside the LayoutRootNode so we can use that for lookups, etc. Rewriting to return LayoutRootNode instead of a list of components.
1. Fixing implementation to take start/stop indexes into account
2. Fixing implementation of getRepeatingGroupStartStopIndex() to work properly when repeating group index is undefined (i.e. when there are no rows in the group).
3. Making the type from flat() smarter, depending on if groups are returned or just plain components
4. Implementing parents() and isHidden(), now used in validation.ts
5. Removing the old version of this code in validation.ts and replacing it with nodesInLayout(). The dataModelBindings are now correctly set in the recursive functions, so no need to pass on `groupDataBinding` and `index` any longer.
…n control the edit.addButton boolean value using a layout expression looking up a plain value in the data model. Lots of stuff left (I added a few TODOs). This is touching GroupContainer.tsx fairly lightly, resolving the 'container.edit' object using 'useLayoutExpressions'. Implementing LayoutRootNodeCollection to make it easier to work with multiple layouts.
…n here just returns the input, but this can be used to lookup values from dataModel/component/etc)
…'m supposed to call it). I started implementing this before we had a meeting today, where one of the points brought up was that we might want app developers to specify the index placeholders (like in textResourceBindings wih variables) instead, possibly making this redundant.
# Conflicts:
#	src/altinn-app-frontend/src/utils/validation/validation.ts
…g type-support for providing components with resolved layout expressions
… and 'true', my equals function failed without this.
…sionContext.ts class (which I'll add to in order to implement proper error messages when expressions fail to validate)
… looking up values (that are called the same way as other functions). This is a large step forwards in being able to support proper recursive expressions. Quite a few error test cases missing now, along with pretty error messages when an expression fails during validation and/or runtime evaluation.
@Altinn Altinn deleted a comment from lgtm-com bot Oct 20, 2022
@olemartinorg olemartinorg marked this pull request as ready for review October 20, 2022 11:02
Copy link
Contributor

@Magnusrm Magnusrm left a comment

Choose a reason for hiding this comment

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

Nicely done! 👑
If i have managed to understand at least some of this correctly, flattening the layout seems to allow for a simpler solution with validation-message handling as well as for the new expression-handling?

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.

Good work 🥇

# Conflicts:
#	src/altinn-app-frontend/src/features/form/containers/GroupContainer.tsx
#	src/altinn-app-frontend/src/features/form/containers/RepeatingGroupTable.tsx
@lgtm-com
Copy link
Contributor

lgtm-com bot commented Oct 24, 2022

This pull request fixes 1 alert when merging 9fd63c8 into 2c413a1 - view on LGTM.com

fixed alerts:

  • 1 for Useless conditional

…when conditionalRendering fetched the state before it had been set correctly
@lgtm-com
Copy link
Contributor

lgtm-com bot commented Oct 24, 2022

This pull request fixes 1 alert when merging 082634e into 03184e4 - view on LGTM.com

fixed alerts:

  • 1 for Useless conditional

Ole Martin Handeland added 5 commits October 25, 2022 11:49
…icate objects failed to preserve the rowIndex value, which in turn caused a cypress test to fail(!). Yay for cypress!
…using start/stop indexes. This caused the likert.js cypress test to fail, as the rowIndex was fetched from the array index in the `rows` property, leading to the wrong language keys being fetched. Again, thank goodness for the cypress tests!
…ix did not account for direct usage of useExpressions() failing to memoize.
@github-actions github-actions bot added area/language related to language and texts area/test related to automated and functional testing labels Oct 26, 2022
@lgtm-com
Copy link
Contributor

lgtm-com bot commented Oct 26, 2022

This pull request fixes 1 alert when merging 7dae397 into 21b496b - view on LGTM.com

fixed alerts:

  • 1 for Useless conditional

@sonarqubecloud
Copy link

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 4 Code Smells

63.7% 63.7% Coverage
0.0% 0.0% Duplication

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/language related to language and texts 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.

Logic/expressions in layout
4 participants