Page MenuHomePhabricator

Deleting statement panels
Closed, ResolvedPublic

Description

User story

As a Commons contributor, I want to be able to delete an entire statements panel, so that I can ensure quality of metadata being added to files.

We have this

Currently for the Depicts panel, there is a "Remove all" button that removes all the added statements to a panel. The reason it's working this way now is because the Depicts panel itself cannot be deleted by users, but we are allowing for the batch deletion of all added statement values (and their qualifiers).

However, with the addition of all other statements, this gets a little confusing, because we need to support the use case of deleting entire statement panels (for example, deleting an incorrect statement due to the evolving data modeling of statements that the community is engaged in). Because we need to support this use case, it becomes confusing to offer a "Remove all" button option on the Depicts panel.

We want this

  • For the Depicts panel to not show the "Remove all" button at all
  • For the behavior of the "Remove all" button for all statements (except Depicts) to delete the entire panel

Acceptance Criteria:

  • "Remove all" button doesn't show for Depicts panel
  • "Remove all" button deletes the statements panel for all statements except Depicts

Details

Related Gerrit Patches:
mediawiki/extensions/WikibaseMediaInfo : masterConfirmation msg for 'remove all' on File page
mediawiki/extensions/WikibaseMediaInfo : masterOnly show 'remove all' if property is non-default

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMay 23 2019, 9:20 PM
Ramsey-WMF triaged this task as Medium priority.May 29 2019, 7:22 PM
Ramsey-WMF added a project: SDC Design.
Ramsey-WMF moved this task from Backlog 📚 to Doing 📝 on the SDC Design board.
Ramsey-WMF assigned this task to Cparle.Jun 26 2019, 8:01 PM

Tentatively assigning to Cormac for now, for after we move other statements to test Commons.

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

Cparle added a comment.Jul 4 2019, 2:30 PM

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

@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

Cparle reassigned this task from Cparle to Edtadros.Jul 9 2019, 4:12 PM
Cparle added a subscriber: Cparle.

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.

egardner claimed this task.Jul 22 2019, 4:31 PM
egardner moved this task from Needs QA to To Do on the Structured-Data-Team-Current-Work board.
egardner added a subscriber: Edtadros.

@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.

egardner removed egardner as the assignee of this task.Jul 24 2019, 4:31 PM
egardner added a subscriber: egardner.
Ramsey-WMF closed this task as Resolved.Jul 31 2019, 9:02 PM
Ramsey-WMF claimed this task.

Works as currently specced. We may need to revisit when we consolidate the UW and FP editing flows, particularly if there ends up being no more "Publish Changes" buttons per panel.