Skip to content
This repository was archived by the owner on Mar 25, 2021. It is now read-only.

Improve lookup props#142

Merged
micgro42 merged 3 commits into
masterfrom
addMissingProps
Jan 13, 2021
Merged

Improve lookup props#142
micgro42 merged 3 commits into
masterfrom
addMissingProps

Conversation

@micgro42
Copy link
Copy Markdown
Collaborator

@micgro42 micgro42 commented Jan 8, 2021

(This should be merged into master after #141 and followed by #123)

It makes more sense to translate the error message only once in the EntityLookup component and not two times in the two components that use it.

Also, this adds the missing disabled prop to the EntityLookup and ItemValueLookup. The PropertyLookup is never disabled and therefore doesn't need it.

We were already doing that implicitly, because the existing
updateProperty action is already being called with null as the property
without typescript reflecting that.

This now extracts that functionality into its own action and mutation
making the overall flow simpler.

There are no behavioral changes in this commit.
This way, the interfaces between StringValueInput and ItemValueLookup
are nicely aligned which will make the next step easier.
@micgro42 micgro42 force-pushed the extractStringValueInput branch from 47a5b47 to 9a7a184 Compare January 12, 2021 14:17
It makes more sense to translate the error message only once in the
EntityLookup component and not two times in the two components that use
it.

Also, this adds the missing disabled prop to the EntityLookup and
ItemValueLookup. The PropertyLookup is never disabled and therefore
doesn't need it.
@micgro42 micgro42 force-pushed the extractStringValueInput branch from 9a7a184 to b93788f Compare January 13, 2021 10:27
Base automatically changed from extractStringValueInput to master January 13, 2021 11:18
@micgro42 micgro42 merged commit 324f0e0 into master Jan 13, 2021
@micgro42 micgro42 deleted the addMissingProps branch January 13, 2021 11:18
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants