Change 520035 had a related patch set uploaded (by Cparle; owner: Cparle):
[mediawiki/extensions/WikibaseMediaInfo@master] Only show 'remove all' if property is non-default

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

We may need to add a confirmation model that asks something like: "This action will delete all existing values for this property. Properties with no values will be removed when changes are published. Are you sure?" @PDrouin-WMF to provide further guidance

I think that might be hand-holding the user a little too much, but will defer to @PDrouin-WMF

@PDrouin-WMF any thoughts?

@Cparle - Agree with Ramsey here that there needs to be a confirmation model. I'm assuming this kind of action won't be taken often (deleting all values, including the property) but I do think there needs to be sufficient warning that property and anything underneath it will be deleted.

However, @Ramsey-WMF is proposing using "Remove all" to remove all the values, and the property itself will be deleted upon publishing. I had imagined "Remove all" removing all the values, as well as the property itself (both for FP and UW). I'd prefer the latter. Is that possible though?

I guess it is, but that'd effectively mean there'd be no 'publish' step for 'remove all' - it'd just warn you when you clicked it and remove-and-publish when you click 'ok'

Okay, thanks @Cparle - never mind then. Ramsey's solution would work best. I think the text does a good job of explaining what'll happen.

Change 520035 merged by jenkins-bot:
[mediawiki/extensions/WikibaseMediaInfo@master] Only show 'remove all' if property is non-default

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

Change 521461 had a related patch set uploaded (by Cparle; owner: Cparle):
[mediawiki/extensions/WikibaseMediaInfo@master] Confirmation msg for 'remove all' on File page

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

Change 521461 merged by jenkins-bot:
[mediawiki/extensions/WikibaseMediaInfo@master] Confirmation msg for 'remove all' on File page

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

Hey @Ramsey-WMF, we have a problem with removing properties. While @Edtadros might come across this in testing, I thought I'd write about it and include steps to reproduce the issue.

Deleting valueless properties

Essentially, once a property has been chosen and added, there is no way to delete a valueless property on File Page. It's also an issue on UW, though it's slightly mitigated by having just one Publish button, as I'll explain below.

When a user attempts to "Remove all" on File Page, they will see a modal that warns: "This action will delete all existing values for this property. Properties with no values will be removed when changes are published." While "Remove all" currently deletes all values for that property, it does not delete the property itself, and I believe it should.

The "Remove all" button should be present as soon as a property has been added, and it should delete the property as well as any values it may or may not have. Users should still see a modal that warns them that "This action will delete this property, as well as any values for that property."

Flow and how to reproduce it

Steps to reproduce:

• Select “Add statement” button (note the presence of the trashcan, the search box can be deleted at this point)
• Select search box
• Begin typing “instance of” (note the lack of “Remove all” button)
• Select search box
• Begin typing “gigabyte”
• Select “Remove all” (note the modal — it is impossible to publish Properties with no values, therefore they can never be removed)
• Select “OK” (note the lack of “Remove all” again)

This is also a problem on Upload Wizard. However, where the modal text says "This action will delete all existing values for this property. Properties with no values will be removed when changes are published." is true, because if a user adds at least one value for one property, the Publish data button becomes active and the user is able to publish what's been added, and any valueless properties are removed during that process. Therefore, just one Publish button mitigates the issue slightly, but it's still not a good experience.

Recommendation

I'd strongly recommend us consider making some adjustments to make the Learn more link and the Remove button more obvious, both on FP and UW. Here's one approach:

As we've discussed before, this approach is fine. But we may need to combine with a way to visually distinguish the "not required but strongly recommended" properties like Depicts. That will help explain why depicts (and perhaps other statement panels in the future) don't have remove links.

That should be another ticket, we still need the "Remove all" functionality to remove the property itself.

Visually distinguishing recommended properties like Depicts would be best handled with design iteration & user testing.

@PDrouin-WMF I think this really should be a separate ticket. AFAICS your original acceptance criteria have been met

Hey @Cparle I know this ticket has become rather gnarly, but I don't think the original AC has been met:

"Remove all" button deletes the statements panel for all statements except Depicts

^This is the part that isn't working. Remove all doesn't delete any statement's entire panel, including the property itself. I think due to me not being explicit in the description that it is hard to decipher that I meant the panel's property too, and that it should be able to be deleted even if it has no values. That being said, do I need to revise the description/AC or write a new ticket?

In the discussion of this ticket we agreed (https://phabricator.wikimedia.org/T224249#5313187) that 'remove all' would remove the values, and the property would be removed on publish. A patch to do that has been merged and released, so IMO we need a new ticket

Also FWIW ... I don't like the 'remove all' -> confirm -> publish flow. To me it'd be more intuitive for a click on the 'confirm' popover to actually kick off the publish step

Ok, I'm removing myself from this ticket but I'll take on the new one once it goes up.