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

feat(v2): edit checkbox field in form builder #3581

Merged
merged 16 commits into from
Mar 21, 2022

Conversation

mantariksh
Copy link
Contributor

@mantariksh mantariksh commented Mar 16, 2022

This PR reimplements the checkbox field editor originally implemented in #3131, with the refactors from #3547 included. There are a few significant differences from the original implementation:

  • The design of the checkbox editor has changed from two tabs to a single tab containing all the fields.
  • The original ValidationOptions validation did not allow one of customMin or customMax to be set without setting the other. This PR fixes this.

Known issues

  1. When the sidebar drawer is closed, clicking on a field results in the drawer opening to the field list, rather than directly to the editing page for that field. This will be addressed separately. fixed in fix: ensure clicking field opens to field editor #3583

Next steps

Field duplication and deletion will be implemented together with the relevant mocks before submitting the form builder for design review.

While design review is happening, I will concurrently work on the edit pages for the other fields.

Divider between drawer elements was not extending all the way to the
drawer edge as per the design.
This commit contains a couple of significant changes.

First, useEditFieldForm is updated to take a transform prop, which
allows the shape of the form inputs to be different from the field.
This allows e.g. for checkbox field options to be read as a string
from the form, then transformed into a string[] in the field.

Second, the useEffect in useEditFieldForm is removed. Its purpose
was to ensure that the edit field drawer correctly rerendered when
the field being edited changed, but it was implicitly relying on
useDebounce to be called first in order to work properly. Instead,
a (still hacky but) more explicit implementation using component
keys was chosen, which is explained in the code comments.
This CSS property was causing the content of the drawer to wrongly
overflow in the x direction.
The form reset was causing a bug where if customMax or customMin
was undefined, the field would be saved correctly, but the UI would
show the customMin or customMax value reverting to its previous value.
Copy link
Contributor

@karrui karrui left a comment

Choose a reason for hiding this comment

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

lgtm, much cleaner!

@mantariksh mantariksh merged commit 88aba27 into form-v2/develop Mar 21, 2022
@mantariksh mantariksh deleted the form-v2/edit-checkbox branch March 21, 2022 08:10
@justynoh justynoh mentioned this pull request Oct 5, 2022
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.

2 participants