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.

Sign up for GitHub

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

Jump to bottom

Support FEEL for Multiselect Values #679

Merged
merged 2 commits into from
Jun 15, 2023
Merged

Support FEEL for Multiselect Values #679

merged 2 commits into from
Jun 15, 2023

Conversation

pinussilvestrus
Copy link
Contributor

@pinussilvestrus pinussilvestrus commented Jun 6, 2023 •
edited
Loading

Closes #673

  • Adds valuesExpression property to populate multiselect options via FEEL
  • Adds AutoFocusSelectEntry to focus another entry after select change

image

Demo: https://demo-673-values-feel--camunda-form-playground.netlify.app/

@bpmn-io-tasks bpmn-io-tasks bot added the in progress Currently worked on label Jun 6, 2023
@github-actions github-actions bot temporarily deployed to demo-673-values-feel June 6, 2023 06:45 Destroyed
@github-actions github-actions bot temporarily deployed to demo-673-values-feel June 6, 2023 06:45 Destroyed
@github-actions github-actions bot temporarily deployed to demo-673-values-feel June 6, 2023 09:42 Destroyed
@github-actions github-actions bot temporarily deployed to demo-673-values-feel June 12, 2023 08:59 Destroyed
@github-actions github-actions bot temporarily deployed to demo-673-values-feel June 12, 2023 09:23 Destroyed
@github-actions github-actions bot temporarily deployed to demo-673-values-feel June 12, 2023 11:53 Destroyed
@github-actions github-actions bot temporarily deployed to demo-673-values-feel June 12, 2023 12:49 Destroyed
Base automatically changed from 653-adorners-images to develop June 13, 2023 06:54
@github-actions github-actions bot temporarily deployed to demo-673-values-feel June 13, 2023 07:00 Destroyed
@pinussilvestrus pinussilvestrus marked this pull request as ready for review June 13, 2023 08:19
@bpmn-io-tasks bpmn-io-tasks bot added needs review Review pending and removed in progress Currently worked on labels Jun 13, 2023
@github-actions github-actions bot temporarily deployed to demo-673-values-feel June 13, 2023 08:20 Destroyed
@pinussilvestrus
Copy link
Contributor Author

This is ready for review now.

RomanKostka
RomanKostka approved these changes Jun 13, 2023
View reviewed changes
Copy link

@RomanKostka RomanKostka 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 to me.
One comment regarding the deletion of entries when i switch the options source:
We could show a warning to the users when they have already added options and want to switch to another option source.

Right now i loose all of my entered options when doing so.
With a warning we could reduce frustration -> not sure if this is a painpoint at all - i just noticed it

vsgoulart
vsgoulart approved these changes Jun 13, 2023
View reviewed changes
packages/form-js-viewer/src/render/hooks/useValuesAsync.js Outdated Show resolved Hide resolved
packages/form-js-editor/src/features/properties-panel/components/AutoFocusSelect.js
Comment on lines +29 to +31
// @Note(pinussilvestrus): There is an issue in the properties
// panel so we have to wait a bit before showing the entry.
// Cf. https://github.com/camunda/linting/blob/4f5328e2722f73ae60ae584c5f576eaec3999cb2/lib/modeler/Linting.js#L37
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we have an issue to link to?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately, not yet. I will see that I'll create one

@github-actions github-actions bot temporarily deployed to demo-673-values-feel June 15, 2023 06:53 Destroyed
@pinussilvestrus
Copy link
Contributor Author

We could show a warning to the users when they have already added options and want to switch to another option source.

Right now i loose all of my entered options when doing so. With a warning we could reduce frustration -> not sure if this is a painpoint at all - i just noticed it

I think this is a general pattern in our tooling, e.g. BPMN: when we switch element types, there are quite some properties that will be removed (cf. integration tests). I think the general answer to this is to ensure undo and redo is working. That's not the case in the demo, but should work in the modeler products.

Let's keep an eye on that.

@pinussilvestrus pinussilvestrus merged commit 7134dd0 into develop Jun 15, 2023
@pinussilvestrus pinussilvestrus deleted the 673-values-feel branch June 15, 2023 07:27
@bpmn-io-tasks bpmn-io-tasks bot removed the needs review Review pending label Jun 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vsgoulart vsgoulart vsgoulart approved these changes

@RomanKostka RomanKostka RomanKostka approved these changes

@christian-konrad christian-konrad Awaiting requested review from christian-konrad

@Skaiir Skaiir Awaiting requested review from Skaiir

Assignees

@pinussilvestrus pinussilvestrus

Labels
deploy-preview
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support FEEL for multiselect values
3 participants
@pinussilvestrus @vsgoulart @RomanKostka