Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PICARD-2681: Update text messages for new user and file save confirmation dialogs #2255

Merged
merged 4 commits into from
Jul 11, 2023

Conversation

rdswift
Copy link
Collaborator

@rdswift rdswift commented Jul 7, 2023

Summary

  • This is a…
    • Bug fix
    • Feature addition
    • Refactoring
    • Minor / simple change (like a typo)
    • Other
  • Describe this change in 1-2 sentences: Update the text messages in the new user and file save confirmation dialogs.

Problem

Discussion in the Community Forum suggested some improvements to the text displayed in the new user and file save confirmation dialogs.

Solution

Update text messages in the dialogs.

Action

No additional action required.

@rdswift rdswift requested a review from phw July 7, 2023 22:26
@rdswift
Copy link
Collaborator Author

rdswift commented Jul 8, 2023

The revised new user dialog looks like:

image

In the ticket, @Aerozol has suggested it be changed to:

image

Personally, I think that's a bit too brief. Perhaps it should be something in between. Comments?

@Sophist-UK
Copy link
Contributor

Sophist-UK commented Jul 8, 2023

  1. I don't think the text should be all warnings and negativity. There needs to be something positive to start with which the current text has.
  2. I don't particularly like the word "undesired changes" and would prefer "unwanted changes" (or more ominously "disastrous changes").

But there is some scope for shortening. How about:

New User Warning

Picard overwrites tags and moves/renames your music files using metadata from Musicbrainz. It is highly flexible, but configure it wrongly and you can make disastrous changes to your music files. Changes you save cannot easily be reversed.

We strongly recommend that:

  1. Before you use Picard, you understand the documentation.
  2. You test using copies of your music files in small batches.

Picard is open-source software written by volunteers. You use Picard at your own risk.

[✔️] Disastrous saves? When I restart Picard, remind me again how not to make-it-so.

@Sophist-UK
Copy link
Contributor

P.S. Do we want to do something similar in the File Save warning pop-up?

@rdswift
Copy link
Collaborator Author

rdswift commented Jul 8, 2023

P.S. Do we want to do something similar in the File Save warning pop-up?

Here is the updated file save confirmation dialog:

image

Personally, I think it's fine.

@rdswift
Copy link
Collaborator Author

rdswift commented Jul 8, 2023

2. I don't particularly like the word "undesired changes" and would prefer "unwanted changes" (or more ominously "disastrous changes").

Softening it a bit, perhaps "unexpected changes"?

zas
zas previously approved these changes Jul 8, 2023
@Sophist-UK
Copy link
Contributor

Sophist-UK commented Jul 9, 2023

  1. Should the File Save Warning bullet points start with a capital letter?
  2. Can we combine the words before and after the bullet points into a single phrase. That might mean a change of tense i.e.

File Save Warning

You are about to save XX files and this will:

  • Overwrite existing metadata (tags) within the files;
  • Rename the files; and
  • Move the files to a different location.

This action cannot be undone. Do you want to continue?

[ ] Don't show this warning again

  1. As a very minor visual point, I think the file save warning dialog needs a slightly larger right margin.
  2. For both dialogs, I am not certain that the ordered/unordered lists really need to be indented because the numbers and bullets already create a visual indent for the associated text. This could make the pop-ups a little more compact.

P.S. @rdswift Bob said "Softening it a bit, perhaps 'unexpected changes'?" Personally I think "unwanted changes" is better as one of my intents was to consistently make it clear that the user is responsible for whatever changes get made, not Picard.

P.P.S. Should we be more consistent in the use of "Make it so!" and use it wherever we currently use "Ok"? (I am not sure what happens to "Make it so" in translations.)

@rdswift
Copy link
Collaborator Author

rdswift commented Jul 9, 2023

  1. Should the File Save Warning bullet points start with a capital letter?

That violates the style guide. For what it's worth, I don't completely agree with the way the guide handles lists by only including a period at the end of the final bullet but not including a semicolon (or comma) at the end of the preceding lines.

  1. Can we combine the words before and after the bullet points into a single phrase.

I agree that reads better in English, but it can become a nightmare to set it up for translation because it needs to be able to handle singular (only one file), plural (more than one file), as well as the case where the number of files is not specified. As @zas can attest, I struggle with setting up the translations properly, although there may be a simple solution that I'm not seeing. The current code accommodates all three cases with minimal translation effort required.

  1. As a very minor visual point, I think the file save warning dialog needs a slightly larger right margin.

I noticed that too, but the display is generated automatically by Qt. That allows it to accommodate different line lengths and such due to translation into other languages. I can't think of an easy fix.

  1. For both dialogs, I am not certain that the ordered/unordered lists really need to be indented because the numbers and bullets already create a visual indent for the associated text. This could make the pop-ups a little more compact.

Again, this formatting is handled automatically by Qt using the supported HTML codes.

P.S. @rdswift Bob said "Softening it a bit, perhaps 'unexpected changes'?" Personally I think "unwanted changes" is better as one of my intents was to consistently make it clear that the user is responsible for whatever changes get made, not Picard.

I guess I don't understand the difference between your preference of "unwanted" versus the current "undesired". Don't they mean the same thing?

P.P.S. Should we be more consistent in the use of "Make it so!" and use it wherever we currently use "Ok"? (I am not sure what happens to "Make it so" in translations.)

That's an interesting thought. However, in my opinion that is a separate topic of discussion outside the scope of this PR.

@Sophist-UK
Copy link
Contributor

That violates the style guide. For what it's worth, I don't completely agree with the way the guide handles lists by only including a period at the end of the final bullet but not including a semicolon (or comma) at the end of the preceding lines.

  1. Those are MB editing style guidelines, so not sure whether or not they apply to Picard. And Picard violate them where we want to anyway (e.g. use of "make it so" instead of "OK".
  2. IMO common grammatical rules of English (or other languages) should take precedence.
  3. Those said, I fully understand why you think we should stick to the MB style guidelines.

@Sophist-UK
Copy link
Contributor

I agree that reads better in English, but it can become a nightmare to set it up for translation because it needs to be able to handle singular (only one file), plural (more than one file), as well as the case where the number of files is not specified. As @zas can attest, I struggle with setting up the translations properly, although there may be a simple solution that I'm not seeing. The current code accommodates all three cases with minimal translation effort required.

How about "You are about to save XX file(s) and this will:"

That said, the traditional way of dealing with zero/singular/multiple/unknown numbers of things is to have separate translation strings for each, but that sacrifices translation effort for user friendliness.

@Sophist-UK
Copy link
Contributor

I guess I don't understand the difference between your preference of "unwanted" versus the current "undesired". Don't they mean the same thing?

In my head, "unwanted" means the user got it wrong whereas "undesired" means that Picard got it wrong. Or unwanted means I asked for X but got Y, whereas "undesired" means in my head I was expecting X but I never stated it explicitly and received Y. But that may just be me.

@rdswift
Copy link
Collaborator Author

rdswift commented Jul 9, 2023

How about "You are about to save XX file(s) and this will:"

I'm okay with that, and actually did some code changes locally to see how it looked. I see that it is done that way in a few other places in Picard. If there is consensus with the other reviewers @zas and @phw, I'll make the change.

That said, the traditional way of dealing with zero/singular/multiple/unknown numbers of things is to have separate translation strings for each, but that sacrifices translation effort for user friendliness.

I looked into that as well, but it appears that Picard is not set up to use the ngettext method of gettext that provides for translating plural strings separately. I could do it within the code to specify which translation string to use, but that becomes a problem with some languages that have multiple different plural translations depending on the number.

I'm thinking that the "proper" way to do it would be to add ngettext support to Picard, but again that is outside the scope of this PR.

@rdswift
Copy link
Collaborator Author

rdswift commented Jul 9, 2023

That violates the style guide. For what it's worth, I don't completely agree with the way the guide handles lists by only including a period at the end of the final bullet but not including a semicolon (or comma) at the end of the preceding lines.

  1. Those are MB editing style guidelines, so not sure whether or not they apply to Picard.

Actually, this style guide is one that @Aerozol recently put together covering all MetaBrainz development work. It is different from the MusicBrainz editing style guide(s).

@Sophist-UK
Copy link
Contributor

So, what editing process did this style guide go through? I am probably not plugged into the Metabrainz discussions enough and probably missed it, but if I had spotted it I would almost certainly have commented that lists should follow the grammatic standards of the language concerned - though upon further thought I suspect that the lack of punctuation is a lowest-common-denominator thing that makes translations easier because you don't have to deal with the extra level of complexity for idiosyncrasies of how semi-colons are used or not used in languages other than English i.e. like the numbers thing mentioned earlier.

@phw
Copy link
Member

phw commented Jul 10, 2023

Personally, I think that's a bit too brief. Perhaps it should be something in between. Comments?

Something in between sounds good, but not sure about the exact text to use. For me the current and revised texts are a bit too much a wall of text and also too much negativity. I agree on that part with @Sophist-UK , it should not sound like using Picard is a terrible idea :)

What about:

Changes made by Picard are not reversible.

Picard is a very flexible music tagging tool which can change your file's tags and rename them. We strongly recommend that you:

  • read the Picard documentation (also accessible from the Help menu)
  • test with copies of your music and work in small batches

Picard is open source software written by volunteers. It is provided as-is and with no warranty.

[ ] Show this message again the next time you start Picard.

Trying to still include some of the details of the current message, but bee briefer so the central points are easier to see.

I looked into that as well, but it appears that Picard is not set up to use the ngettext method of gettext that provides for translating plural strings separately.

Yes, ngettext is the proper way to handle string changes based on file number. That cleanly handles languages with more forms then just singular and plural. It's already used at various places, e.g. look at collection.py where it is used like this:

status_msg = ngettext(
    'Added %(count)i release to collection "%(name)s"',
    'Added %(count)i releases to collection "%(name)s"',
    count)

For what it's worth, I don't completely agree with the way the guide handles lists by only including a period at the end of the final bullet but not including a semicolon (or comma) at the end of the preceding lines.

Please don't add that final period. I don't think this is needed and we can and should ignore the guideline here. Just appending the dot will break with translations in multiple ways:

  • in the easiest case it will append this extra period and it will look out of place in the translation if that is uncommon there
  • it might also end up in double period, if it is common that list items end with a period in the translation and hence the translator added a period to every item
  • it might lead to mix up of different symbols not common for the script being used, e.g. for Chinese or Japanese

To handle that properly we would need two variants of each string, and it would just be extra work and also be very confusing to translators.

Can we combine the words before and after the bullet points into a single phrase.

We really should aim to decrease the visual and textual complexity in these dialogs. This will increase it. I have also never seen a list written like this.

Beside visual and textual complexity it also increases programmatic and especially translation complexity. To properly handle this each string would need to exist in 3 variations (last item, second last item and other position). This is extra work and also hard to explain to translators. Many translations likely would not need this and would need to fill all three forms with the same string. Many translators likely would still try to accommodate the English spelling and create something that looks out-of-place in their language.

Let's just not do this and keep it simple. Let's not artificially increase the complexity of a short three-items list.

@Sophist-UK
Copy link
Contributor

Sophist-UK commented Jul 10, 2023

Picard is a very flexible music tagging tool which can change your file's tags and rename them. We strongly recommend that you:

It is not that Picard "can change" your file's tags, it is explicitly designed to overwrite them with the metadata from MusicBrainz (unless you explicitly configure it not except specific tags). The purpose of this dialog is to warn people (especially those that have already spent a lot of manual effort tagging and renaming their files) that Picard is going to overwrite the "whole bally lot" i.e. "boom" (with apologies to those who don't get the Blackadder references even though this tool is not named Baldrick).

It seems to me that toning down the warning goes against it's purpose, and as I have already suggested IMO we should also make it crystal clear that the responsibility for making sure that it doesn't do something the user doesn't want it to is the user's - and they should not go ballistic and complain about the tool afterwards.

Re: lists - on reflection it seems to me that the most important thing about lists is that they are readable and get the point across. Given the difficulties of getting translations done, I think that punctuation perfection should be sacrificed for ease of translation - and this probably means keeping it as simple as it can possibly be.

@phw
Copy link
Member

phw commented Jul 10, 2023

It is not that Picard "can change" your file's tags, it is explicitly designed to overwrite them with the metadata from MusicBrainz

I think I don't really understand what the difference between changing and overwriting should be in this context. But if "overwriting" makes it more clear then that's fine to use as well.

@Sophist-UK
Copy link
Contributor

"overwrite" means change all data wholesale, "change" means something less. Given the use case this is aimed at (i.e. users who complain when Picard overwrites their hand crafted metadata, using "overwrite" seems to me to get the message across better.

@rdswift
Copy link
Collaborator Author

rdswift commented Jul 10, 2023

Okay, I've tried to take into account all of the discussion and have (currently) landed on the following screens:

New User Warning

image

Save Confirmation (one file)

image

Save Confirmation (multiple files)

image

In addition to properly handling the plural cases for the File Save confirmation dialog, I also set the default button to Cancel to help avoid accidental acceptance.

@Sophist-UK
Copy link
Contributor

Personally I think the File Save warning is perfect.

However I would propose some minor tweaks to the New User warning that make the message a bit stronger as follows:

New User Warning

Changes you save using Picard cannot be undone.

Picard is a very flexible music tagging tool which uses MusicBrainz data to set tags and rename files. We strongly recommend that you:

  • read the User Guide (also available from the Help menu)
  • test with copies of your music files and work in small batches

Picard is open-source software written by volunteers. You use Picard at your own risk.

☑️ Show this warning next time you start Picard.

Rationale for changes:

  1. This is pretty much the same length as the current proposal, but it is a little stronger in tone.
  2. Make it clear that responsibility for changes is on the user not the software.
  3. Tags will definitely be overwritten using Musicbrainz data not just possibly.
  4. Replace legalese with another plain language reminder about personal responsibility.
  5. Repeat that it is a WARNING not just any old message.

@rdswift
Copy link
Collaborator Author

rdswift commented Jul 10, 2023

Changes you save using Picard cannot be undone.

I have no real objections to this sort of change, although I also have no objection to the way it currently reads. If we were to change it, I might reword it slightly to say, "Changes you make using Picard cannot be undone."

Picard is a very flexible music tagging tool which uses MusicBrainz data to set tags and rename files. We strongly recommend that you:

This is not necessarily correct because the user can make changes without using MusicBrainz data by changing the tag values in the tag editor and saving from the clustering pane. Perhaps something like, "Picard is a very flexible tool which sets music file tags and renames the files. We strongly recommend that you:"

Picard is open-source software written by volunteers. You use Picard at your own risk.

Again, I have no real objections to this change, although I also have no objection to the way it currently reads.

@Sophist-UK
Copy link
Contributor

Sophist-UK commented Jul 10, 2023

This is not necessarily correct because the user can make changes without using MusicBrainz data by changing the tag values in the tag editor and saving from the clustering pane.

It is precisely because Picard is a very flexible music tagging tool, that almost any statement you make has its exceptions. Yes you can manually change the tag values, and you can script them too, and you can except certain tags from being overwritten, etc. etc. etc. But this is both for New Users and a warning (rather than detailed advice for Expert Users) and, like teaching children to be safe, we need to generalise and simplify the message to what is largely true in the default configuration. Out of the box, Picard overwrites pretty much all tags with what gets downloaded from MusicBrainz, and making this simplified but blunt statement will make sure that people understand that if they have spent man weeks manually applying tags these will all (or mostly) get overwritten (which is exactly the example use case that kicked off this functionality).

@Sophist-UK
Copy link
Contributor

If we were to change it, I might reword it slightly to say, "Changes you make using Picard cannot be undone."

It is the act of saving that does the damage - and this is an explicit act by a user separate from simply loading the files into Picard - which is why I explicitly chose to use the word save.

However, in the end my views are one person - and I am happy to go along with whatever the majority / consensus is.

@Aerozol
Copy link
Contributor

Aerozol commented Jul 11, 2023

So, what editing process did this style guide go through?

Sorry for the late reply, I’m on holiday atm.

The styleguide is based on Digital.govt.nz (which has a CC license), which was in turn based on gov.uk standards. There were some changes to our version, after it was circulated via team meetings and IRC for quite a while (Only Reo really got stuck in, if I remember correctly).

Because the focus is on accepted readability and accessibility standards the scope for community input on some of it is limited, but I am happy to review any specific feedback and re-circulate :)

picard/ui/savewarningdialog.py Outdated Show resolved Hide resolved
@rdswift rdswift merged commit 4e9e54d into metabrainz:master Jul 11, 2023
52 checks passed
@rdswift rdswift deleted the update_dialog_text branch July 11, 2023 16:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants