appsmithorg / appsmith

Platform to build admin panels, internal tools, and dashboards. Integrates with 25+ databases and any API.
https://www.appsmith.com
Apache License 2.0
34.2k stars 3.7k forks source link

[Feature] Internal property to detect changes in a form #4182

Closed somangshu closed 2 years ago

somangshu commented 3 years ago

Summary

As a user, I need an internal property in the form to detect any changes made to the form elements

Motivation

"I would want to implement something to alert users before changing page with unsaved changes. But to do that I am missing a feature on the form widget: properties that would be like form_1.has_changes , that would be true if any of the fields have been manually changed after loading its default value."

dilippitchika commented 2 years ago

@somangshu this sounds simple on paper, like we can add a "form.hasChanges" property to all form widgets. Need some help scoping this from your side, have the following questions for now

  1. Is this high effort?
  2. What are the edge cases?
vicky-primathon commented 2 years ago

Not all widgets has isDirty property which is there in Input Widget, DropDown Widget, Phone & Currency Input widget. We first need to add the same property for Date Widget, Rich Text Editor, Checkbox, etc. widgets. That we can use the same to compute hasChanges property in Form widget. cc. @somangshu

dilippitchika commented 2 years ago

Hey @vicky-primathon thanks for taking a look, can you discuss the effort needed for this once with @somangshu and update this thread? We can then take a call on this

somangshu commented 2 years ago

@riodeuno @sbalaji1192 looping you guys in, Does the above solution by @vicky-primathon make sense here? Would you suggest a separate approach?

sbalaji1192 commented 2 years ago

@somangshu @vicky-primathon Yeah. this seems like a good solution. we are already updating form data property by taking values from child widgets. However, reading isDirty won't be correct, since it just tells whether we have edited the input or not. if the user has edited input and later reinstated the same value, we shouldn't consider that scenario as having unchanged changes. I guess we can only check widget.defaultValue !== widget.value or something along the line.

somangshu commented 2 years ago

@sbalaji1192 I have a questions:

sbalaji1192 commented 2 years ago

@somangshu defaultValue is the property that holds the default value of the widget. In input widget, it's defaultText.

somangshu commented 2 years ago

@sbalaji1192 apologies, but I am still confused, Does this property set with the meta value? I am assuming that this is only set with change in the defaultText property for say in the input widget

vicky-primathon commented 2 years ago
  1. Need to find changes using above logic for below widgets: Checkbox Group Widget Checkbox Widget Currency Input Widget Date Picker Widget v1, v2 Dropdown Widget FilePicker Widget Input Widget v1, v2 Multi Select Tree Widget Multi Select Widget Phone Input Widget Radio Group Widget Rich Text Editor Widget Single Select Tree Widget Switch Group Widget Switch Widget Table Widget
  2. Need to handle the change logic for above widget in Form Widget.
  3. Expose hasChange property as form widget API.
dilippitchika commented 2 years ago

@vicky-primathon why do we need table widget here?

vicky-primathon commented 2 years ago

@dilippitchika , To detect when row of a table inside Form widget gets selected/deselected. If it is not required this can be dropped. Please suggest.

riodeuno commented 2 years ago

Any widget which has meta properties (particularly properties which change based on user input) can be technically, configured to be used in a form widget. To recognise if a form is dirty or touched, we need these properties in the individual widgets.

As @vicky-primathon has correctly identified, we must add these derived properties in these widgets, before adding this derived property in the form widget.

Regarding the Table Widget, we can skip this and concern ourselves with widgets which have been noticed in a form, in the wild. @dilippitchika please help in identifying such widgets from the list shared by @vicky-primathon.

A thumb rule I would consider is -- If a widget has one (and only one), user provided value and one (and only one) associated default value, we should add a dirty/touched property to the widget.

dilippitchika commented 2 years ago

We don't need the table widget for now.

Adding a few more widgets which can be used in forms now

  1. Audio recorder
  2. Camera widget

Not sure if we can add map widget, but that can also be present in forms where users can choose a location.

MichaelLeeHobbs commented 2 years ago

Sorry to make this more complex. Maybe consider this for a phase 2 to not hold up the basic isDirty.

List Widget on form has the following cases:

  1. A list item could have input fields that have been changed
  2. A list item could have been removed
  3. A new list item could have been added

Story: We have Medical Group Form that has a Sites List. Users can "Add Site", "Delete Site", or "Edit Site". Each item on the "Sites List" has SiteNameInput and SiteCodeInput.

Suggestion: I do not think you were ever be able to account for all cases where a form should be marked dirty. To avoid endlessly making updates to handle new edge cases simply add a easy way to mark a form as dirty. Form.isDirty = true or Form.setDirty(true|false)

somangshu commented 2 years ago

@MichaelLeeHobbs thanks for bringing us the amazing context, we really appreciate your inputs. While I start a discussion thread for this internally, We are building a feature called as JSON form > #2504, which allows you to use datasource / JSON objects to build form easily, We handle many configurations out of the box for you and also allow you to setup more customisation, We have a preview available for this for you to try it out and let us know any feedback you might have, If this solves your problem, You will be glad to know that we are in the process of shipping this out soon.

cc @ashit-rath

MichaelLeeHobbs commented 2 years ago

@somangshu sounds very interesting but when I try to login with my github account to the preview I get sent to https://release.app.appsmith.com/applications/user/login?error=true An automatic form builder based on JSON would be amazing.

somangshu commented 2 years ago

@MichaelLeeHobbs you will have to use dummy login credentials, use the sign up button. Oauth does not work on previews. Please get in touch on discord if you need my test credentials.

somangshu commented 2 years ago

@Tooluloope @wmdev0808 was it notified to the user-education-pod about this change, this will need documentation change for sure.

cc @ajinkyakulkarni @Pranay105 @dilippitchika

MarionToutant commented 2 years ago

Hi @dilippitchika, when will this feature be released to end-users?

dilippitchika commented 2 years ago

Hello @MarionToutant this is already live in v.1.6.20 and is available for everyone