Bug Hunting Session
Bug 111318 - Improve message about pivot table/chart when removing a cell
Summary: Improve message about pivot table/chart when removing a cell
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Calc (show other bugs)
Version:
(earliest affected)
6.0.0.0.alpha0+
Hardware: All All
: medium enhancement
Assignee: Not Assigned
URL: http://nabble.documentfoundation.org/...
Whiteboard: target:6.0.0
Keywords:
Depends on:
Blocks:
 
Reported: 2017-08-03 20:11 UTC by Julien Nabet
Modified: 2017-08-10 21:34 UTC (History)
5 users (show)

See Also:
Crash report or crash signature:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Julien Nabet 2017-08-03 20:11:15 UTC
Description:
With master sources updated today, on Calc there's this specific message when deleting a cell from pivot table with 1 or several pivot charts associated:
"There is at least one pivot chart associated with this pivot table. Should remove all or abort?"

In French, the message is:
"Il y a au moins un diagramme dynamique associé avec cette table dynamique. Tout doit-il est supprimé ou abandonner ?" (so with syntax problem).

The goal of this bugtracker is to discuss and improve the message.


Steps to Reproduce:
Not relevant here

Actual Results:  
"There is at least one pivot chart associated with this pivot table. Should remove all or abort?"


Expected Results:
To discuss


Reproducible: Always

User Profile Reset: No

Additional Info:


User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Firefox/52.0
Comment 1 Julien Nabet 2017-08-03 20:23:43 UTC
Sophie/Jean-Baptiste: I put you in cc (in addition with Tomaž) because the French translation is incorrect. I don't know if we should change it right now or first, wait for a better original message.
Comment 2 Jean-Baptiste Faure 2017-08-03 20:41:03 UTC
Thank you Julien. I changed the French translation of the second sentence in "Tout doit-il être supprimé ou faut-il abandonner ?"

I will change again if we get a better original message.

Best regards. JBF
Comment 3 Pedro 2017-08-03 21:30:37 UTC
(In reply to Julien Nabet from comment #1)

I'm not a native English speaker either but in my opinion

"There is at least one pivot chart associated with this pivot table. Should remove all or abort?"

could be replaced by

"Deleting this pivot table will also remove the pivot chart(s) associated with it. Continue?" Yes/No

(my French isn't good enough to suggest a translation :) )
Comment 4 Julien Nabet 2017-08-03 21:44:39 UTC
(In reply to Pedro from comment #3)
> ...
> "Deleting this pivot table will also remove the pivot chart(s) associated
> with it. Continue?" Yes/No
> ...
The problem is the initial user action is to remove a cell not delete pivot table (and even less + delete associated chart pivot) so a user may not see (perhaps would fill a tracker) the relation between his/her action and this message.

Perhaps something like this?
"This action will also remove the whole pivot chart and associated pivot charts. Do you agree to proceed?"
(or "Do you want to proceed?")
(not too long, indicate all the elements: initial action and its consequences)
Comment 5 Pedro 2017-08-03 22:17:39 UTC
(In reply to Julien Nabet from comment #4)
> Perhaps something like this?
> "This action will also remove the whole pivot chart and associated pivot
> charts. Do you agree to proceed?"
> (or "Do you want to proceed?")
> (not too long, indicate all the elements: initial action and its
> consequences)

How about

"Deleting this cell(s) will also remove the whole pivot table and associated pivot
charts. Do you want to proceed?"
Comment 6 Julien Nabet 2017-08-04 05:37:36 UTC
(In reply to Pedro from comment #5)
> How about
> "Deleting this cell(s) will also remove the whole pivot table and associated
> pivot
> charts. Do you want to proceed?"
The problem is the plural of "this cell" is "these cells", so "(s)" isn't sufficient.
That's why I used "This action"
Comment 7 Pedro 2017-08-04 08:49:35 UTC
(In reply to Julien Nabet from comment #6)
> The problem is the plural of "this cell" is "these cells", so "(s)" isn't
> sufficient.
> That's why I used "This action"

I am aware of the plural rules but thought it wasn't a big issue. As you prefer ;)
Comment 8 Adolfo Jayme 2017-08-04 18:43:26 UTC
By replacing one word in Pedro’s suggestion you can avoid the pluralization problem entirely:

> Deleting this pivot table will also remove *any* pivot charts associated with it.
Comment 9 Julien Nabet 2017-08-05 16:21:38 UTC
(In reply to Adolfo Jayme from comment #8)
> By replacing one word in Pedro’s suggestion you can avoid the pluralization
> problem entirely:
> 
> > Deleting this pivot table will also remove *any* pivot charts associated with it.

Since we were talking about plural of cells, it could be:
"Deleting any cells of a pivot table will also remove it and any associated pivot
charts. Do you want to proceed?"
Comment 10 Heiko Tietze 2017-08-08 11:40:00 UTC
(In reply to Adolfo Jayme from comment #8)
> Deleting this pivot table will also remove all associated pivot charts.

The shorter the better. But in any case I think it's clear to the user.

(removing needsux)
Comment 11 Julien Nabet 2017-08-08 12:05:06 UTC
Following last Heiko's comment, the message could also be:
"Deleting any cells of a pivot table will also remove it and all associated pivot
charts. Do you want to proceed?" So "all associated" instead of "any associated", it would avoid "any" repeat.

Does this last suggestion could be ok or someone disagrees?
Comment 12 Julien Nabet 2017-08-08 17:10:30 UTC
I submit a patch to review here:
https://gerrit.libreoffice.org/#/c/40897/
don't hesitate to comment/change it.

Pedro: your email wasn't known on gerrit (or I missed it) so couldn't add you.
Comment 13 Commit Notification 2017-08-08 18:47:45 UTC
Julien Nabet committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=fac539889e32e5e47ca6b6da0a588abbe4cbe066

tdf#111318: Improve message about pivot table/chart when removing a cell

It will be available in 6.0.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 14 Tomaz Vajngerl 2017-08-09 06:40:10 UTC
Why "cells"? Seems superfluous to mention cells.
Comment 15 Heiko Tietze 2017-08-09 06:44:32 UTC Comment hidden (off-topic)
Comment 16 Julien Nabet 2017-08-09 07:02:13 UTC
Don't hesitate to update the message.
I tried to (In reply to Tomaz Vajngerl from comment #14)
> Why "cells"? Seems superfluous to mention cells.

Because the initial action here is to delete at least one cell not the pivot table.
What message do you suggest?
Comment 17 sophie 2017-08-09 09:04:10 UTC
(In reply to Julien Nabet from comment #11)
> Following last Heiko's comment, the message could also be:
> "Deleting any cells of a pivot table will also remove it and all associated
> pivot
> charts. Do you want to proceed?" So "all associated" instead of "any
> associated", it would avoid "any" repeat.
> 
> Does this last suggestion could be ok or someone disagrees?

Thank you Julien, the message is clearer now, +1 on my side - Sophie
Comment 18 Tomaz Vajngerl 2017-08-09 11:25:36 UTC
(In reply to Julien Nabet from comment #16)
> Because the initial action here is to delete at least one cell not the pivot
> table.
> What message do you suggest?

The context menu with the "delete" is already for the whole pivot table. By the time you get this message it should be already established that the action is for the whole pivot table not a cell action, no matter where they right-clicked. 

Also we don't warn that the delete action will delete the whole pivot table, so why do we need to mention this here? The main message is about that the associated pivot chart will have to be deleted if we choose to proceed, nothing else needs to be clarified IMHO.
Comment 19 Julien Nabet 2017-08-09 18:55:51 UTC
(In reply to Tomaz Vajngerl from comment #18)
> (In reply to Julien Nabet from comment #16)
> > Because the initial action here is to delete at least one cell not the pivot
> > table.
> > What message do you suggest?
> 
> The context menu with the "delete" is already for the whole pivot table. By
> the time you get this message it should be already established that the
> action is for the whole pivot table not a cell action, no matter where they
> right-clicked.
I retested it to see again the popup.
There are indeed 2 groups separated by a separator:
- Cut, Copy and Paste
- Edit Layout..., Refresh, Filter... and Delete

First group concerns 1 or n cells.
The second group can be understood as "pivot table action" since "Edit Layout..." can't apply to anything else.

So I understand better why you and other people consider that Delete concerns implicitely the whole pivot table.

If we start with this basis, we could indeed simplify the message into this:
"Deleting the pivot table will also remove any associated pivot charts.\nDo you want to proceed?"

Is it better?
Comment 20 Tomaz Vajngerl 2017-08-09 20:48:02 UTC
Perfect, thanks
Comment 21 Tomaz Vajngerl 2017-08-09 20:52:58 UTC
Em... if it is not clear that the context menu items refer to the whole pivot table, maybe we should think about how to improve that. But that's a different issue altogether.
Comment 22 Julien Nabet 2017-08-10 05:28:00 UTC
(In reply to Tomaz Vajngerl from comment #21)
> Em... if it is not clear that the context menu items refer to the whole
> pivot table, maybe we should think about how to improve that. But that's a
> different issue altogether.

About this, perhaps we should replace "Delete" by "Delete pivot table". But it's indeed another point.

Heiko/Sophie/Jean-Baptiste/Pedro: what do you think about comment 19?
Comment 23 Heiko Tietze 2017-08-10 07:29:12 UTC
(In reply to Julien Nabet from comment #19)
> "Deleting the pivot table will also remove any associated pivot charts.\nDo
> you want to proceed?"

Sounds good to me, in particular with the following confirmation.

(In reply to Julien Nabet from comment #22)
> About this, perhaps we should replace "Delete" by "Delete pivot table". But
> it's indeed another point.

Makes sense, or "Delete table" to have it even shorter. In general, I think it's not a big thing because the user can undo the deletion.
Comment 24 sophie 2017-08-10 08:50:29 UTC
(In reply to Heiko Tietze from comment #23)
> (In reply to Julien Nabet from comment #19)
> > "Deleting the pivot table will also remove any associated pivot charts.\nDo
> > you want to proceed?"
> 
> Sounds good to me, in particular with the following confirmation.

yes, +1 on my side
> 
> (In reply to Julien Nabet from comment #22)
> > About this, perhaps we should replace "Delete" by "Delete pivot table". But
> > it's indeed another point.
> 
> Makes sense, or "Delete table" to have it even shorter. In general, I think
> it's not a big thing because the user can undo the deletion.

"Delete Table" is again ambiguous, I much prefer to be explicit here and use "Delete Pivot Table" - Sophie
Comment 25 Commit Notification 2017-08-10 17:08:18 UTC
Julien Nabet committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=ec779d811ab5fcb0957f362086fe7856219bdff2

Revert "tdf#111318: Improve message about pivot table/chart when removing a cell"

It will be available in 6.0.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 26 Julien Nabet 2017-08-10 17:11:32 UTC
Here's the new patch for the initial problem:
https://gerrit.libreoffice.org/#/c/40991/

I'm waiting for verification with Jenkins and I'll push it.

About "Delete pivot table" instead of "Delete", perhaps a new bugtracker?
Comment 27 Jean-Baptiste Faure 2017-08-10 20:32:01 UTC
(In reply to Julien Nabet from comment #22)
> [...]
> Heiko/Sophie/Jean-Baptiste/Pedro: what do you think about comment 19?

Sound good to me too.

Best regards. JBF
Comment 28 Commit Notification 2017-08-10 21:32:41 UTC
Julien Nabet committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=4e2b44860c2c304ea728c512b47ca07aaf1cd452

tdf#111318: Improve msg about pivot table/chart when removing a cell (take 2)

It will be available in 6.0.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 29 Julien Nabet 2017-08-10 21:34:49 UTC
Let's put this one to FIXED now.