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

Support for editable states and alias expressions for fields #3645

Open
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

VitorVieiraZ
Copy link
Contributor

@VitorVieiraZ VitorVieiraZ commented Oct 16, 2024

Screen.Recording.2024-10-17.at.08.28.44.mov

Resolves #2607, Resolves #3494

@VitorVieiraZ VitorVieiraZ changed the title WIP - Support for editable states and alias expressions for fields Support for editable states and alias expressions for fields Oct 17, 2024
Copy link

github-actions bot commented Oct 17, 2024

Pull Request Test Coverage Report for Build 11978385757

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 161 unchanged lines in 4 files lost coverage.
  • Overall coverage increased (+0.02%) to 60.432%

Files with Coverage Reduction New Missed Lines %
input/core/merginuserinfo.cpp 1 75.18%
input/app/position/providers/simulatedpositionprovider.cpp 1 90.91%
input/app/attributes/attributedata.cpp 12 91.24%
input/app/attributes/attributecontroller.cpp 147 76.59%
Totals Coverage Status
Change from base Build 11601124075: 0.02%
Covered Lines: 7913
Relevant Lines: 13094

💛 - Coveralls

Copy link
Collaborator

@tomasMizera tomasMizera left a comment

Choose a reason for hiding this comment

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

Nice :) I have just a bunch of small comments, but generally it looks good 👍🏻 @uclaros can I ask you to have a look here please?

app/attributes/attributecontroller.cpp Show resolved Hide resolved
app/attributes/attributedata.h Outdated Show resolved Hide resolved
app/attributes/attributedata.h Show resolved Hide resolved
app/attributes/attributecontroller.cpp Outdated Show resolved Hide resolved
app/attributes/attributecontroller.cpp Outdated Show resolved Hide resolved
app/attributes/attributedata.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

@uclaros uclaros left a comment

Choose a reason for hiding this comment

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

Looks good!
I'd love some docstrings for the new items in FormItem.
Maybe add some comments to the new private members, as I see that's the only thing that's documented already :)

Comment on lines 268 to +269
!isReadOnly,
editableExpression,
Copy link
Collaborator

Choose a reason for hiding this comment

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

We are still missing the logic when there is "default value on update enabled". In that case we do not want to set the field to be editable. Let's not read the expression in case field.defaultValueDefinition().applyOnUpdate() is set (see line https://github.com/MerginMaps/mobile/pull/3645/files#diff-1cac530c00b6b3114d6f3da9d8dc0fefd2a486a7eefc58814e9e8dd1c0da69acR255)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Issue resolved:
(Test2 with default value on update enabled is not editable even when Editable expression returns true)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Alias as expression is not evaluated in the form Support for editable state of a field
4 participants