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

FEATURE: Add button to delete unused tags #6587

Merged
merged 1 commit into from
Nov 12, 2018
Merged

Conversation

davidtaylorhq
Copy link
Member

This is particularly useful if you have uploaded a CSV file, and wish to bulk-delete all of the tags that you uploaded.

Adds an option to the tag administration hamburger:
screenshot 2018-11-09 at 16 58 41

Clicking the button presents a confirmation modal, including the number of tags which will be deleted:
screenshot 2018-11-09 at 16 55 22

@discoursebot
Copy link

You've signed the CLA, davidtaylorhq. Thank you! This pull request is ready for review.

@@ -2707,6 +2707,10 @@ en:
upload_description: "Upload a text file to create tags in bulk"
upload_instructions: "One per line, optionally with a tag group in the format 'tag_name,tag_group'."
upload_successful: "Tags uploaded successfully"
delete_unused_confirmation: "Are you sure you want to delete %{count} unused tags?"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs pluralization with one and other keys.

@tgxworld
Copy link
Contributor

I'm wondering if it would be better to list the names of the unused tags instead of just displaying the count.

@davidtaylorhq
Copy link
Member Author

Thanks for the feedback. I have made the changes.

  • Tags are listed in the confirmation:

screenshot 2018-11-12 at 15 16 54

  • If there are more than 20, it says "and # more"

screenshot 2018-11-12 at 15 11 47

  • Strings have 'one' and 'many' entires. @gschlager can you check that looks ok? There are two numbers in the delete_unused_confirmation_more string, but {{total}} will always be greater than 1

@ZogStriP
Copy link
Member

  • If there are more than 20, it says "and # more"

😍

This is particularly useful if you have uploaded a CSV file, and wish
to bulk-delete all of the tags that you uploaded.
@davidtaylorhq davidtaylorhq merged commit d89ffbe into master Nov 12, 2018
other: "{{count}} tags will be deleted: %{tags}"
delete_unused_confirmation_more:
one: "{{total}} tags will be deleted: %{tags} and one more"
other: "{{total}} tags will be deleted: %{tags} and %{count} more"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately 2 numbers won't work for languages like Russian where you have other categories than one and other. For example, if total is 31 it would require the one category for {{total}} and the many category for %{count}.

Possible solutions:

  1. Use Message Format which is totally awesome, but super hard to translate.
  2. Split up the translation. I think you could get away with that option in this case by using the following strings:
      delete_unused_confirmation:
        one: "%{count} tag will be deleted: %{tags}"
        other: "%{count} tags will be deleted: %{tags}"
      delete_unused_confirmation_more_tags:
        one: "%{tags} and one more"
        other: "%{tags} and %{count} more"

And use it like this:

  const string = I18n.t("tagging.delete_unused_confirmation", {
    count: tags.length,
    tags:
      more === 0
        ? tagString
        : I18n.t("tagging.delete_unused_confirmation_more_tags", {
            count: tags.length,
            tags: tagString
          })
  });

💡 Two more tips:

  1. {{key}} is deprecated. Use %{key} whenever possible.
  2. Instead of using the number 1 inside the one category, it's good practice to always use %{count}. It makes it easier for translators who always need to use %{count} for their language. See Always use %{count} variable when translating pluralized strings.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Amazing, thanks for the info @gschlager - I've made those changes in ba00fcc (sorry I got a bit trigger-happy with the merge button!)

@davidtaylorhq davidtaylorhq deleted the delete-unused-tags branch November 12, 2018 16:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

6 participants