search

bpmn-io / form-js

View and visually edit JSON-based forms.
https://bpmn.io/toolkit/form-js/
Other
414 stars 107 forks source link

Support dynamic forms with form field data population #200

Closed MaxTru closed 2 years ago

MaxTru commented 2 years ago

This is an epic issue that may be broken down into multiple tasks if needed

What should we do?

Our users (e.g. SUPPORT-12025) require forms to be created dynamically based on input data, for example depending on a category, display specific form fields. Dynamic forms allow you to automatically create form fields and layouts based on ingesting data from any source.

This does not mean that we're going to add data transformation capabilities in form-js (as of now). The library itself only provides a simple in-out API to provide the necessary data and pipe it out afterward.

Prerequisites

Cf. Research Miro Board

Breakdown

Other topics (related and follow up, but not in the scope of this epic)

Why should we do it?

Be able to create forms dynamically on given data.

Context: https://github.com/camunda/product-hub/issues/55 Cf. Value Proposition Cf. https://jira.camunda.com/browse/SUPPORT-12025

MaxTru commented 2 years ago

Comment from @marstamm in Modeler Weekly:

Comment from @nikku in Modeler Weekly in response:

marstamm commented 2 years ago

Possible Platform integration

The Platform also supports a json variable type. The JSON is delivered Stringified from the API and has to be parsed in the frontend. See this example of a variable result from the backend:

JSON ```Json { "VariableA": { "type": "Json", "value": "[\n{\"a\": 1},\n{\"b\": 2}\n]", "valueInfo": {} }, "VariableB": { "type": "Integer", "value": 1234, "valueInfo": {} } } ```

We can use the "Json" type to create lists and use them in the form. Keep in mind that:

pinussilvestrus commented 2 years ago

Sync of next working steps (@Skaiir @pinussilvestrus)

Skaiir commented 2 years ago

Just had a chat with @vsgoulart who is our link to C8 tasklist and had some pretty valuable takeaways from it.

Following discussion: Implementing this ticket just feels like a no-brainer and everybody's agreed so far, so I've adopted this into the scope of dynamic forms.

Skaiir commented 2 years ago

Insights from C7 tasklist by @tasso94

nikku commented 2 years ago

A little aside related to the previous point, the fact that we don't output falsy values for most of our single select controls could also be considered a limitation.

Don't we? Can you share an example we can dive into?

Skaiir commented 2 years ago

Radio buttons for example:

image

nikku commented 2 years ago

A little aside related to the previous point, the fact that we don't output falsy values for most of our single select controls could also be considered a limitation.

How are these an example?

nikku commented 2 years ago

Like for a radio, what shall be the none value? Null initially? My intuition is that radio has "must be one of them" semantics.

Skaiir commented 2 years ago

Well, perhaps we're not understanding eachother but the point that was raised is that the selectable values defined statically inside of the form is lost to the process. A radio button is a singleselect. We don't output information about the options which were not selected. This just ties in with the point right before and is something Tassilo brought up. That information is restricted to our form and there's no way to access it from the process down the line.

Skaiir commented 2 years ago

This obviously depends on use case and it might just be better to do it the way we're currently doing it, I just brought it up as a thought.

nikku commented 2 years ago

Ah, so the the point is that we want to submit all results (by checked and unchecked)?

nikku commented 2 years ago

Example scenario: A multi checkbox with 10 different emails, we select 3 of those, those 3 get an in depth email about some process information, the 7 others only get a quick summary. If those were defined in the form options, the process scope is not aware of it, and hence we'd need to output the falsy selection as well to create the above described behavior.

For this example (which relates to falsy values) I'd argue that

(1) a list of X emails is a process variable (or otherwise hardcoded engine binding); we'll support mapping such dynamic process variables to selects options (2) returning only what is selected is safe, as at run-time can diff between what is being returned from the form and what has previously been fetched

Again, I may miss-understand something. In that case it could be best to actually model such scenario, ensure we all understand it, and carefully decide if we want to change existing semantics (start to submit falsy stuff).

Skaiir commented 2 years ago

Is there a way to do (2) if the data isn't fetched but hardcoded into the form definition?

nikku commented 2 years ago

But yea, if multi-select becomes a dedicated, new component (not the same as a check-box) we can be more creative with submit behaviors.

Is there a way to do (2) if the data isn't fetched but hardcoded into the form?

We could add the ability to submit additional static configuration with the form. That would be one resolution.

Like: Always submit emailUsers = [ { name, value=email } ] and selectedEmailUsers=[...].

It may be worth discussing how far we want to go with static bindings, and also what to consciously not support for improved happiness of the 80%.

Skaiir commented 2 years ago

Agreed, the point only related to static definitions anyways. I agree completely that if you've got your data source as a process variable there's no argument there we wouldn't need it. Cool, we'll chat about it during the Kickoff.

felix-mueller commented 2 years ago

Hi there

I got a support feature request (https://jira.camunda.com/browse/SUPPORT-12403) where a user is asking about support for:

Given: { "schemaVersion": 2, "exporter": { "name": "form-js (https://demo.bpmn.io)", "version": "0.1.0" } , "components": [ { "text": "# File an Invoice\n\nAdd your invoice details below.\n\nCustomer Id: ${customerId}", "type": "text", "id": "Field_1m5gj8s" }, { "action": "submit", "key": "submit", "label": "Submit", "type": "button", "id": "Field_03qrh3a" } ], "type": "default", "id": "Form_1ef51ca" }

Where ${customerId} get's replaced by the process variables

Is this in scope of this ticket here?

nikku commented 2 years ago

No it is not, or at least not planed for.

It is related to https://github.com/bpmn-io/form-js/issues/247 and I've linked the support case there.

Maybe it makes sense to investigate it in the course of our dynamic form endeavours @christian-konrad @Skaiir? I've heart this request just yesterday, and a couple of times in the past, too.

felix-mueller commented 2 years ago

Thank you for the quick reply!

pinussilvestrus commented 2 years ago

Thanks for sharing, I also added it to the "other topics" section above (not in this epic, but as a follow-up) 👍

Skaiir commented 2 years ago

@nikku We're going to bring FEEL into forms, and this looks like that would fit such an implementation as well. I think this could be taken on as part of the conditional rendering epic here: https://github.com/camunda/product-hub/issues/56.

Perhaps it's not the perfect epic for this but it's more related than dynamic select data.

Skaiir commented 2 years ago

Next release will be October the 11th, which gives us a timeline of 5.5 months for these changes to be completed. (Not that this much is needed)

pinussilvestrus commented 2 years ago

Kickoff 29th April 2022

For more details, refer to this miro board.

Decisions

image

Action items

image

dfulgham commented 2 years ago

Why wouldn't dynamic input data just be created prior to instantiating the form, passing a function into the form as input data would be considerably harder to control for security reasons, making it easy to inject malicious code?

Unless you are suggesting to allow the function to access internal form data, for example to accomplish cascading selects, one select determines the values of the next. We would then have some default params passed to the the functions like formData etc.

inputData = {
  countryValues: [{label:"Canada",key:"CA"}, {"label":"Germany","key":"DE"}, ...],
  citiesValues: async function(formData){ 
     const {country} = formData;
     const cities = await fetch(...);
     return cities.filter(c => { return c.country === country}).map(c => {
       return { label: c.name, value: c.key }
    })
   },
...
}

triggering all functions run when formData changes somehow, and also trigger a loading indicator when the promise is waiting to resolve would be advantageous.

This would be cool, but concerns of security issues remain.

EDIT: it would be even better if we could establish a memoization and run the function only when specific formData values changes. for example when formData.country changes:


inputData = {
   countryValues: [{label:"Canada",key:"CA"}, {"label":"Germany","key":"DE"}, ...],
   citiesValues: [["country"], async function(country){ ... do something with country when it changes and return new values ...}]

}
dfulgham commented 2 years ago

Example scenario: A multi checkbox with 10 different emails, we select 3 of those, those 3 get an in depth email about some process information, the 7 others only get a quick summary. If those were defined in the form options, the process scope is not aware of it, and hence we'd need to output the falsy selection as well to create the above described behavior.

For this example (which relates to falsy values) I'd argue that

(1) a list of X emails is a process variable (or otherwise hardcoded engine binding); we'll support mapping such dynamic process variables to selects options (2) returning only what is selected is safe, as at run-time can diff between what is being returned from the form and what has previously been fetched

Again, I may miss-understand something. In that case it could be best to actually model such scenario, ensure we all understand it, and carefully decide if we want to change existing semantics (start to submit falsy stuff).

Wouldn't it just be better to handle that in the process, the process would have the initial list of email and now the selected few. by removing the selected emails from the full list process variable and store it in a filtered process variable list in the process and then send summary emails to those, and detailed emails to the selected values. No other form select works by submitting not selected values.

dfulgham commented 2 years ago

Well, perhaps we're not understanding eachother but the point that was raised is that the selectable values defined statically inside of the form is lost to the process. A radio button is a singleselect. We don't output information about the options which were not selected. This just ties in with the point right before and is something Tassilo brought up. That information is restricted to our form and there's no way to access it from the process down the line.

I think a better way of passing data from input to output would be to create a hidden type form component, this way you could pass any value from the input to the output if required. This would be in line with how a regular web form would pass data that doesn't require user interaction. Bind the select and the hidden to the same input property, the select component would pass the selected values and the hidden component would pass the complete list.

dfulgham commented 2 years ago

But yea, if multi-select becomes a dedicated, new component (not the same as a check-box) we can be more creative with submit behaviors.

Is there a way to do (2) if the data isn't fetched but hardcoded into the form?

We could add the ability to submit additional static configuration with the form. That would be one resolution.

Like: Always submit emailUsers = [ { name, value=email } ] and selectedEmailUsers=[...].

It may be worth discussing how far we want to go with static bindings, and also what to consciously not support for improved happiness of the 80%.

Could we create an option in the form schema to provide a preSubmit function that could arbitrarily structure the submitted data any way the developer would like prior to form submission? Passing this function the formData, InputData, and perhaps components

nikku commented 2 years ago

I think a better way of passing data from input to output would be to create a hidden type form component (https://github.com/bpmn-io/form-js/issues/200#issuecomment-1123170808)

@dfulgham This is what we settled on (@pinussilvestrus @Skaiir please confirm).

Wouldn't it just be better to handle that in the process, the process would have the initial list of email and now the selected few (https://github.com/bpmn-io/form-js/issues/200#issuecomment-1123167429)

@dfulgham Exactly. We'll not persue this direction (@pinussilvestrus @Skaiir please confirm).

Generally speaking we shall not re-invent the web, but rather build in the spirit of existing web forms (including hidden inputs you mentioned for secret data passing). This is what we do to date, this is what we'll do in the future.

nikku commented 2 years ago

Could we create an option in the form schema to provide a preSubmit function that could arbitrarily structure the submitted data any way the developer would like prior to form submission?

What would be the dedicated use-case? As you mentioned I can absolutely wrap the form and do whatever I want before import, and after submit.

nikku commented 2 years ago

Why wouldn't dynamic input data just be created prior to instantiating the form (https://github.com/bpmn-io/form-js/issues/200#issuecomment-1123160041)

The sole reason for passing a function would be to allow dynamic lookup, i.e. when huge amounts of data exist that cannot be pre-loaded (i.e. list of emails to auto-complete). In this case you can provide a lookup function.

The default use-case for our dynamic form works is indeed that data (and options) are dynamically populated via input variables, however just-in-time lookup is one of the options we see with popular multi-select libraries, i.e. select2.

dfulgham commented 2 years ago

What would be the dedicated use-case? As you mentioned I can absolutely wrap the form and do whatever I want before import, and after submit. (#200 (comment))

It was for the use case where the developer wanted access to the Component Elements (i.e. statically set value lists) and the Input values at time of submission (i.e. in the case of fetched data).

Just brainstorming ways to accommodate the use case put forward by you @nikku but I think these could also be enabled with the hidden component type, allowing the input data at time of submission to be submitted. And I don't personally think I would use the case of needing any statically set values, if I needed them i'd rather set those values with an input value dynamically.

dfulgham commented 2 years ago

(reply to #200 comment)

Why wouldn't dynamic input data just be created prior to instantiating the form

The sole reason for passing a function would be to allow dynamic lookup, i.e. when huge amounts of data exist that cannot be pre-loaded (i.e. list of emails to auto-complete). In this case you can provide a lookup function.

The default use-case for our dynamic form works is indeed that data (and options) are dynamically populated via input variables, however just-in-time lookup is one of the options we see with popular multi-select libraries, i.e. select2.

Yes, i worked my way to this understanding too while providing the example of cascaded selects

pinussilvestrus commented 2 years ago

But yea, if multi-select becomes a dedicated, new component (not the same as a check-box) we can be more creative with submit behaviors.

Is there a way to do (2) if the data isn't fetched but hardcoded into the form?

We could add the ability to submit additional static configuration with the form. That would be one resolution. Like: Always submit emailUsers = [ { name, value=email } ] and selectedEmailUsers=[...]. It may be worth discussing how far we want to go with static bindings, and also what to consciously not support for improved happiness of the 80%.

Could we create an option in the form schema to provide a preSubmit function that could arbitrarily structure the submitted data any way the developer would like prior to form submission? Passing this function the formData, InputData, and perhaps components

That could deserve an extra issue (if not already existing). Feel free to open one and we can discover this particular use case over there 👍

Skaiir commented 2 years ago

I think a better way of passing data from input to output would be to create a hidden type form component (https://github.com/bpmn-io/form-js/issues/200#issuecomment-1123170808)

@dfulgham This is what we settled on (@pinussilvestrus @Skaiir please confirm).

I wouldn't say we settled on anything as part of this epic with regards to passing data. In the end this raises questions about how we differentiate 'form' scope from 'input' scope, and this all will play into conditional rendering, FEEL, which are later epics for this and hence we'll defer the particular data passing case to then.

A hidden form component to do mappings would be a good way to handle this case though, so would pre-submit actions. These are both good brainstorms and we'll consider them both.

Skaiir commented 2 years ago

Following a sync, I'll provide a little bit of summary / context for @currandwyer :

Current icon set in use

image

text field - number - checkbox (individual) - radio (multiple) select - text (supports markdown and basic html) - button

We also have some unused icons. Here is the whole collection: Form icons.zip

The icons which are blocking progress currently are:

Some icons we might want to implement in the future would be:

Note that at this stage, we are very early in defining the requirements. Some icons will not make it, some are redundant and we might opt for a configuration option instead. These are only for context and we're not looking to produce them yet.

A library contributor has provided their implementation of a Fieldset icon already, I've provided it in the attached collection , if you think it's satisfactory, we can use that straight away. This is not within the scope but I'd love bit of feedback on it 😄.

More context:

Discussion thread on the Taglist The issue regarding the new basic fields

MaxTru commented 2 years ago

FYI @tasso94 => we documented our assumptions for the C7 tasklist integration (see breakdown). Could you please raise your hand in case this is unclear to you? Thanks

  • Our working assumption is as following: C7 developers need to pass or retrieve data for the multi-element form components > by using the json type. https://github.com/bpmn-io/form-js/issues/200#issuecomment-979800606
  • The C7 tasklist glue code / adapter would then transform this into a consumable format for form-js (essentially a JSON List > string)
tasso94 commented 2 years ago

What you documented sounds good to me. 🙂 👍

As part of this, we need to check if the external worker and java developer APIs support JSON data.

The C7 JS as well as the Java Client both support Json typed variables:

pinussilvestrus commented 2 years ago

One update regarding the breakdown: We will handle the form-js-playground integration separately, as it does not only depend on dynamic data anymore.

pinussilvestrus commented 2 years ago

For transparency: We just released an alpha version with the dynamic input data feature included: https://github.com/bpmn-io/form-js/pull/282.

Skaiir commented 2 years ago

For visibility, tracking, linting and documentation concerns have moved over to https://github.com/bpmn-io/internal-docs/issues/596. We will now handle these things as part of release instead of continuously, as it reduces overheads and makes it easier for us to work with our various integrations.

Related: https://github.com/bpmn-io/internal-docs/issues/597

nikku commented 2 years ago

We will now handle these things as part of release instead of continuously, as it reduces overheads and makes it easier for us to work with our various integrations.

When did we decide this? And who decided it?

Skaiir commented 2 years ago

@nikku

This was initially in agreement with myself and @pinussilvestrus. We pitched this internally and @CatalinaMoisuc understands where we're coming from as well. If you want to clarify what it's all about we can have a chat.

CatalinaMoisuc commented 2 years ago

@Skaiir see also my comment here: https://github.com/bpmn-io/internal-docs/issues/597#issuecomment-1188693392.

Also, what I agreed on is that it makes sense to wait with i.e. the tracking and documentation until we clarify with Tasklist their requirements. Your argument in that favour was that: we might need to change implementation for being able to integrate with Tasklist so it makes sense to wait so we don't have to make changes i.e. rewrite the docs.

See below:

To recap and make sure we are on the same page, until Tasklist is ready for these changes you plan to:

  • prepare the implementation to make it compatible with Tasklist (when your contact persons from Tasklist are back)
  • implement potential changes required by Tasklist (when your contact persons from Tasklist are back)
  • prepare docs (depends on the integration with Tasklist - the previous two bullet points - so has to be done after potential changes are implemented)
  • clarify what we should track and prepare that (doesn't have any dependencies, can just me done) And once we have the Tasklist release, we do integration testing and prepare that to release in Modeler.

Now given that @nikku mentioned that Tasklist is also releasing alphas monthly then we don't actually have to wait until October and moreover, if we agree on the approach to take with camunda-form-js in #597, we might even want to release earlier.

pinussilvestrus commented 2 years ago

I wouldn't worry too much about implementation requirements change now, as we clarified the API beforehand in discussions with the respective teams.

I think the point here is that we should not feel blocked in closing epics because we have to wait for certain activities that need at least final confirmation (linting, tracking, as mentioned above). These things can be handled in a separate manner (e.g. via a integration tracking issue).

However, I also agree we have the initiative we are trying to roll on (cf. https://github.com/bpmn-io/internal-docs/issues/597#issuecomment-1188693392). We should all agree as a team on the best approach, also including our users (our product teams).

CatalinaMoisuc commented 2 years ago

Notes for today's(21.07.2022) meeting:

Discuss development & integration strategy for form-js features

Note that:

Next steps:

cc @nikku, @christian-konrad, @Skaiir, @pinussilvestrus, @philippfromme

P.S. feel free to edit if I missed anything important.

CatalinaMoisuc commented 2 years ago

As discussed, @Skaiir as the DRI, please make sure you open issues for the next steps and make the dependencies transparent in this epic.

Skaiir commented 2 years ago

I'll close this topic in favor of the integration ticket which can be followed here (WIP), as this one has become very crowded. I'm going to drive forwards integration of these new features and stay in contact with everybody involved.