Question:
'On checking the checkbox, the field will be set to empty and disabled' -> In the design i see a value/placeholder being set (eg; the default date)?

Ill add the placeholder aswell per design

Change #1187435 had a related patch set uploaded (by Daphne Smit; author: Daphne Smit):

[mediawiki/extensions/WikiLambda@master] Visual Editor: Implement Default Value checkboxe

https://gerrit.wikimedia.org/r/1187435

Some improvements:

• Style-wise, it needs some separation from the checkbox to the next field label
• When the field is empty, and the function is being edited, it should initialize the fields so that the checkbox is ticked.
• There's a strange state now, where the checkbox is unticked, but the value can be left empty. It doesn't make much sense, even if the result is the same. I believe we should initialize fields so that the checkbox is ticked for those that allow default values, and if it's not ticked, it would require a valid input. What do you think? Doesn't it feel weird like this?
• Placeholder value sometimes takes the value of other QID which are not the default one. I haven't figured out what's the exact scenario in which this happens, but you can see it in the demo below (default is Universe, but placeholder shows "dog")

In T402176#11184607, @gengh wrote:

Some improvements:

• Style-wise, it needs some separation from the checkbox to the next field label

Yes! good point will add that.

• When the field is empty, and the function is being edited, it should initialize the fields so that the checkbox is ticked.

AH I was indeed doubting if that should happen. ill add it.

• There's a strange state now, where the checkbox is unticked, but the value can be left empty. It doesn't make much sense, even if the result is the same. I believe we should initialize fields so that the checkbox is ticked for those that allow default values, and if it's not ticked, it would require a valid input. What do you think? Doesn't it feel weird like this?

I also thought this was strange but there wasn't really anything in the ticket about it. so I thought this was just purely a feature to make it more clear there are default values. It makes sense to make the field required if you don't check the box. But I am thinking ... is there a use case actually where a user for example wants to leave the value for a date empty. or a natural language. with the default values you can never have an empty field anymore right. thats intended I guess? I think maybe we should make it invalid when the checkbox is not checked then (when the field allows for default values)

• Placeholder value sometimes takes the value of other QID which are not the default one. I haven't figured out what's the exact scenario in which this happens, but you can see it in the demo below (default is Universe, but placeholder shows "dog")

Oops. I accidentally left a test Qid there: [ Constants.Z_WIKIDATA_ITEM ]: () => mw.config.get( 'wgWikibaseItemId' ) || 'Q144',

On previous points

Style-wise, it needs some separation from the checkbox to the next field label

✅ Done! looks great, thanks!

When the field is empty, and the function is being edited, it should initialize the fields so that the checkbox is ticked.

✅ Working great!

There's a strange state now, where the checkbox is unticked, but the value can be left empty.

✅ Flow makes a lot more sense now, thanks a lot. The marking the checkbox to leave the field empty feels intentional.

Placeholder value sometimes takes the value ... "dog"

😆 classic

There's one new thing. When adding a new function that has types as default values, the fields are initialized to checked. I understand that this is consequence of fixing point 2 (initialization), but initializing to checked should be done only when editing a function. When inserting a new function, the field should be empty, but checkbox unchecked, so that checking the field is intentional and informed. Reading the initial specs, I believe this was the initial intention from this feature design.

That makes total sense indeed. I will check that!

There's some new issue in the enabled/disabled state of the update button. See this demo:

• When editing a function call, if there are no changes, the button should not be enabled. Only when there are changes from the initial state, should the "Apply changes" button be enabled.
• On adding the new "label of item" function, the initial state again shows an enabled "Insert" button even when the checkboxes are unselected and the fields are empty, which is a regression from the previous patch update.

Issues should be handles now

When checking/unchecking the checkbox, we should alter the "done" button and activate it or disactivated depending on whether the state is submittable, but should we validate the field as well?

• We don't validate the field, we just set it as empty as if it were an initial state (patchset 11)

• We also validate the field, marking it directly as invalid as it is empty (patchset 13)

cc @DSmit-WMF @gonyeahialam

I'm moving this to the Design review column before we mark it as closed, so that Gregory has some time to review and give his feedback, given that it's a complex new feature that impacts the user experience.

When checking/unchecking the checkbox, we should alter the "done" button and activate it or disactivated depending on whether the state is submittable, but should we validate the field as well?

Does this only apply when editing an inserted function? @gengh

Change #1187435 merged by jenkins-bot:

[mediawiki/extensions/WikiLambda@master] Visual Editor: Implement Default Value checkbox

https://gerrit.wikimedia.org/r/1187435

Let's go with the 2nd option of showing the error

In T402176#11277113, @gengh wrote:

When checking/unchecking the checkbox, we should alter the "done" button and activate it or disactivated depending on whether the state is submittable, but should we validate the field as well?

• We don't validate the field, we just set it as empty as if it were an initial state (patchset 11)

• We also validate the field, marking it directly as invalid as it is empty (patchset 13)

cc @DSmit-WMF @gonyeahialam