Skip to content

Adds "missing" support for form defaults#4452

Open
tiberiuichim wants to merge 13 commits intomainfrom
defaults_missing
Open

Adds "missing" support for form defaults#4452
tiberiuichim wants to merge 13 commits intomainfrom
defaults_missing

Conversation

@tiberiuichim
Copy link
Copy Markdown
Contributor

No description provided.

@netlify
Copy link
Copy Markdown

netlify Bot commented Mar 2, 2023

Deploy Preview for volto ready!

Name Link
🔨 Latest commit 3bb407d
🔍 Latest deploy log https://app.netlify.com/sites/volto/deploys/6417e2eaba7cd00008452734
😎 Deploy Preview https://deploy-preview-4452--volto.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@cypress
Copy link
Copy Markdown

cypress Bot commented Mar 2, 2023

Passing run #4569 ↗︎

0 489 20 0 Flakiness 0

Details:

Merge branch 'master' into defaults_missing
Project: Volto Commit: 3bb407dbf4
Status: Passed Duration: 11:50 💡
Started: Mar 20, 2023 4:40 AM Ended: Mar 20, 2023 4:52 AM

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.

@tiberiuichim tiberiuichim requested a review from a team March 2, 2023 12:13
Copy link
Copy Markdown
Collaborator

@stevepiercy stevepiercy left a comment

Choose a reason for hiding this comment

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

Hit me up for documenting this one. These concepts are not easy to document, even for native English speakers. Assuming the concepts are similar to the Pylons Project's Colander, I might be able to wordsmith the docs. See https://docs.pylonsproject.org/projects/colander/en/latest/api.html#schema-related

Comment thread docs/source/blocks/editcomponent.md Outdated
Comment thread docs/source/blocks/editcomponent.md Outdated
imply an "optional" field.
- only `missing` is declared: the block form is initialized with the `missing`
value. When the user clears the field, the field is reinitialized with the
`missing` value.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'm curious. Does this follow the same pattern as Colander, which shares its roots with Zope? https://docs.pylonsproject.org/projects/colander/en/latest/api.html#schema-related

We need to add the case when neither missing nor default is defined.

The scenarios also need to be clarified about when reinitialization occurs (when field is cleared or form submitted) and where (client or server side).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think I'm more used to the zope.schema concept, rather then colander.

https://github.com/zopefoundation/zope.schema/blob/89f09bb970bc14f81b724e81174aaaedb8853c3a/src/zope/schema/interfaces.py#L251-L261C9

The major difference to the zope.schema is that the UI is automatically updated based on the schema and user interactions, so it's more a "user interaction" issue rather then data validation

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

OK, that makes a little more sense, I guess kinda like HTML inputs getting their attributes for value and required set and unset?

I would suggest the following, and I have further questions to help me understand and clarify the intent.

Let's make this section a definition list for ease of legibility.

`default` and `missing` are declared
:   The block form is initialized with the `default` value.
    When the user clears the field, the field is reinitialized with the `missing` value.
    This would imply a required field.

`default` only is declared
:   The block form is initialized with the `default` value.
    When the user clears the field, the field remains empty.
    This would imply an optional field.

`missing` only is declared
:   The block form is initialized with the `missing` value.
    When the user clears the field, the field is reinitialized with the `missing` value.

`default` and `missing` are not declared
:   The block form has no initialized value.
    The field is required.

Are "block form" and "field" the same thing or different? In my brain, a form is a collection of one or more fields (a schema) with additional attributes.

Also I am struggling with the term "reinitialized". Does that mean the value is shown to the user in the field and/or its HTML attribute value gets set to the missing value? If so, that seems weird to me that when a user clears a field, then its value is reinitialized. Maybe the value is what gets processed outside the client browser? Please help me understand.

tiberiuichim and others added 2 commits March 2, 2023 16:38
Co-authored-by: Steve Piercy <web@stevepiercy.com>
Co-authored-by: Steve Piercy <web@stevepiercy.com>
@sneridagh sneridagh requested a review from davisagli March 2, 2023 18:31
@tiberiuichim
Copy link
Copy Markdown
Contributor Author

@stevepiercy What can we do about the docs?

Copy link
Copy Markdown
Collaborator

@stevepiercy stevepiercy left a comment

Choose a reason for hiding this comment

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

It might be easier to explain with a picture or video, too. Sometimes words just don't make it clear, even for developers, but especially for the end user.

each field: `default` and `missing`. The `default` value is used to populate
the field with an initial value. The `missing` value should be used when the field
should always have a value (as required).

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Off topic rant: offering suggestions for narrative documentation with arbitrary line breaks sucks. This is one good reason to use one sentence per line.

Anyway, here's my suggestion:

In the block schema definition, you can use the two additional properties for each field: `default` and `missing`.

The `default` value is used to set an initial value for the field.
The user may override this value or clear it.

The `missing` value must be a valid value which becomes the field's value to be submitted when the user clears the field.
With `missing`, the field is optional.
Without `missing`, the field is required.

imply an "optional" field.
- only `missing` is declared: the block form is initialized with the `missing`
value. When the user clears the field, the field is reinitialized with the
`missing` value.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

OK, that makes a little more sense, I guess kinda like HTML inputs getting their attributes for value and required set and unset?

I would suggest the following, and I have further questions to help me understand and clarify the intent.

Let's make this section a definition list for ease of legibility.

`default` and `missing` are declared
:   The block form is initialized with the `default` value.
    When the user clears the field, the field is reinitialized with the `missing` value.
    This would imply a required field.

`default` only is declared
:   The block form is initialized with the `default` value.
    When the user clears the field, the field remains empty.
    This would imply an optional field.

`missing` only is declared
:   The block form is initialized with the `missing` value.
    When the user clears the field, the field is reinitialized with the `missing` value.

`default` and `missing` are not declared
:   The block form has no initialized value.
    The field is required.

Are "block form" and "field" the same thing or different? In my brain, a form is a collection of one or more fields (a schema) with additional attributes.

Also I am struggling with the term "reinitialized". Does that mean the value is shown to the user in the field and/or its HTML attribute value gets set to the missing value? If so, that seems weird to me that when a user clears a field, then its value is reinitialized. Maybe the value is what gets processed outside the client browser? Please help me understand.

@tiberiuichim
Copy link
Copy Markdown
Contributor Author

@stevepiercy many thanks for the suggestions, they're all really good. I'll try to incorporate them.

@tiberiuichim
Copy link
Copy Markdown
Contributor Author

Also I am struggling with the term "reinitialized". Does that mean the value is shown to the user in the field and/or its HTML attribute value gets set to the missing value? If so, that seems weird to me that when a user clears a field, then its value is reinitialized. Maybe the value is what gets processed outside the client browser? Please help me understand.

This is exactly the problem I'm trying to fix with this PR. Right now, without this PR, if I set the default value for a field, when I clear the field, immediately the form is automatically updated by Volto form library and the default value is presented again by the field as its value. This happens in the user's browser. So the effect is that, if the developer sets a default value on a field, the field can't be cleared and left empty.

By introducing the missing property, in addition to the default, we're achieving two things:

  • first, we untangle the default, it will only be used when the form is initialized, if the object data (the thing we're editing in the form) didn't have that value before. So, in my scenario above, if I have the default specified, when I clear the field, the default is no longer "written" back in the field value
  • next, we allow the developer to use the missing property to have the current behavior possible, with a field that will always have some value and can't be "cleared".

@stevepiercy
Copy link
Copy Markdown
Collaborator

Can missing be an empty string, None, or Null?

I assume that "form block" and "field" are the same, so I would replace the former with the latter. I also would replace "re/initialized" with "set".

`default` and `missing` are declared
:   The field's value is set to the `default` value.
    When the user clears the field, the field is set to the `missing` value.
    This would make the field required.

I think using a code example of what actually happens might help, too. For example, if I understand correctly:

Let's say that `default=https://default.com` and `missing=https://missing.com`.
When the field is first displayed to the user, and if the field is an HTML text input, its HTML source would render as follows.

```html
<input name="url" value="https://default.com">
```

This would be displayed to the user as follows.

<input name="url" value="https://default.com">

When the user edits the field, but does not clear it, then the value entered by the user is used for the field.
If the user clears the field, then the field's value will be set to the `missing` value.

```html
<input name="url" value="https://missing.com">
```

This would be displayed to the user as follows.

<input name="url" value="https://missing.com">

This would render in the docs as shown in the screenshot.

Screen Shot 2023-03-02 at 11 50 26 PM

I am still not sure about how missing would actually work, but that's my interpretation at this moment. Please correct me if I am mistaken.

@tiberiuichim
Copy link
Copy Markdown
Contributor Author

@stevepiercy You got it correct. That's the behavior. I'll update the docs with your suggestions. Many thanks!

@sneridagh
Copy link
Copy Markdown
Member

@tiberiuichim is this ready?


In effect, the following situation matrix will occur:

- `default` and `missing` are declared: the block form is initialized with
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice. I just gave it a try on local vanilla volto. When both the values "default" and "missing" are used, it somehow re-initializes the form field with default value. Here's the schema snippet I created for demo field:

demo: {
        title: 'demo',
        widget: 'url',
        default: 'www.default.com',
        missing: 'www.missing.com',
      },

In addition to this, idk if its a glitch or the way it works, but the values let it be "missing" or "default" started to re-appear after clearing them and editing the form second time🤔

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@nileshgulia1 that's the effect of setting missing. If you don't want that effect, don't set the missing.

Btw, this is also the default with current Volto. Try it with this widget:

{
        title: 'demo',
        widget: 'url',
        default: 'www.default.com',
      },

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is how the default behaviour works:

defaults.mp4

@sneridagh
Copy link
Copy Markdown
Member

@tiberiuichim Do you think this is ready?

@davisagli could you please give it a try and review it?

Marked for review during next week sprint.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status
Status: Needs discussion

Development

Successfully merging this pull request may close these issues.

4 participants