#37290 closed defect (bug) (fixed)
Shiny Updates: Different strings for plugins and themes?
Reported by: | pavelevap | Owned by: | ocean90 |
---|---|---|---|
Milestone: | 4.6 | Priority: | high |
Severity: | normal | Version: | 4.6 |
Component: | Upgrade/Install | Keywords: | has-patch |
Focuses: | accessibility | Cc: |
Description
There is only one string "%s installed!" for themes and plugins.
https://core.trac.wordpress.org/browser/trunk/src/wp-includes/script-loader.php?rev=37940#L634
But for example in Czech, we have to use different declension for themes "Twenty Sixteen 1.2 instalována!" and plugins "Hello Dolly 1.0 instalován!". It is the same example as Post and Page differences (which have its own strings). I can try to translate it differently, for example "Instalováno: Twenty Sixteen 1.2", but I am not sure about other languages.
Attachments (5)
Change History (37)
#2
@
8 years ago
- Keywords needs-patch added
- Milestone changed from Awaiting Review to 4.6
We have a different context for plugins and themes, see #27889.
We need to add a context for these new strings as well.
#3
@
8 years ago
37290.patch is what I had in mind, but I could not find where installingLabel
and installedLabel
are used. The only instance I've found is in script-loader.php
.
#4
follow-up:
↓ 6
@
8 years ago
%s was successfully deleted
needs a context as well. In Russian, Czech, and other Slavic languages, "plugin" is masculine and "theme" is feminine.
P.S. If the string is only used for plugins, the translator comment should be updated. Currently it says "%s: Plugin or Theme name".
#5
@
8 years ago
- Owner set to swissspidy
- Priority changed from normal to high
- Status changed from new to assigned
#6
in reply to:
↑ 4
@
8 years ago
Replying to SergeyBiryukov:
%s was successfully deleted
needs a context as well. In Russian, Czech, and other Slavic languages, "plugin" is masculine and "theme" is feminine.
P.S. If the string is only used for plugins, the translator comment should be updated. Currently it says "%s: Plugin or Theme name".
Good point. The string is part of the tmpl-item-deleted-row
JS template in wp_print_update_row_templates()
, which is used for the plugins list table and the network themes list table. This means we need to split the template up into 2 separate ones.
In the future, with i18n via JS, this won't be needed anymore.
#7
@
8 years ago
- Focuses accessibility added
- Keywords has-patch needs-testing added; needs-patch removed
In 37290.diff:
- Add context to all the strings in question.
- Uses
installingLabel
andinstalledLabel
for thearia-label
attributes. Seems like a bug that these weren't used before. - Fixes a spelling mistake in
unknownError
(An unknown error occurred
). - Updates the labels in the fixtures file as well.
Adding the a11y label because of the aria-label
changes.
#8
@
8 years ago
Nitpicking:
'activatePlugin' => is_network_admin() ? __( 'Network Activate' ) : __( 'Activate' ), 'activateTheme' => is_network_admin() ? __( 'Network Enable' ) : __( 'Activate' ),
These strings are fine without a context. They are used without a context elsewhere, so adding a context in this single instance would be inconsistent and would just duplicate the string for no reason.
This ticket was mentioned in Slack in #accessibility by afercia. View the logs.
8 years ago
#10
@
8 years ago
- Focuses accessibility removed
Definitely something that should be addressed, however going to remove the accessibility focus since it's not strictly related to accessibility.
#11
@
8 years ago
- Focuses accessibility added
Sorry, not so fast :) Just noticed the changes to aria-label
:)
#12
@
8 years ago
37290.2.diff removes the context from the activatePlugin and activateTheme strings again.
#13
@
8 years ago
37290.3.diff makes the patch apply cleanly against trunk again.
@SergeyBiryukov: would love your i18n input on that.
@afercia: a11y feedback welcome as well. Most notably, aria-label
is now updated for each step of the plugin install process, i.e. "Install xy" -> "xy installed" -> "Activate xy". Previously, only the button text changed ("Install" -> "Installed" -> "Activate")
#14
@
8 years ago
Looks good to me :) The only thing I've noticed so far is a small inconsistency:
Plugins:
when the link changes in "Activate" the aria label changes in "Activate {plugin-name}"
Themes:
when the link changes in "Activate" the aria label changes in "{theme-name} installed!"
Since there's already a wp.speak.a11y()
message to confirm a successful installation ("Installation completed successfully.") maybe I'd consider to change the Themes aria-label using a text to announce the available action: "Activate {theme-name}".
#15
@
8 years ago
Should we add some more comments for translators that some of these strings are for accessibility? See #29748, it could be helpful in this case, I guess.
#16
follow-up:
↓ 17
@
8 years ago
@afercia I added these in 37290.4.diff. Does that work? Theoretically we could add those attributes to other links as well, but I didn't want to go down that rabbit hole…
@pavelevap That's probably better handled in #29748. We still haven't agreed on the exact wording IIRC.
#17
in reply to:
↑ 16
@
8 years ago
Replying to swissspidy:
@afercia I added these in 37290.4.diff. Does that work?
@swissspidy looks good to me! Maybe just check a translators comment /* translators: %s: Theme name */
in script-loader.php
that seems a leftover.
Theoretically we could add those attributes to other links as well, but I didn't want to go down that rabbit hole…
Maybe better to wait for i18n via JS? Otherwise I'm afraid the amount of code necessary would grow insanely.
#18
@
8 years ago
- Keywords needs-testing removed
Thanks for pointing this out. Indeed a leftover. Also just noticed a wrong indentation in wp-admin/includes/update.php
. We could fix these before commit.
Agree that it makes sense to wait for i18n via JS in case of the other strings.
This ticket was mentioned in Slack in #feature-shinyupdates by swissspidy. View the logs.
8 years ago
#20
@
8 years ago
- Owner changed from swissspidy to ocean90
- Status changed from assigned to reviewing
#25
@
8 years ago
I noticed the same problem for strings related to updating :-( It was not problem before, because it worked only for plugins, but now it is wrong for themes. Should I reopen or create new ticket?
#27
@
8 years ago
Same strings as for installation, probably those 3:
Updating %s... %s updated! %s update failed
It is probably the same problem for other languages? @SergeyBiryukov
Not sure about the patch, probably also templates will have to change a little bit. I can look into it tomorrow...
#28
@
8 years ago
@pavelevap: I've double-checked, and these strings are only used for plugins, not for themes:
There are currently 3 strings that are used for both plugins and themes:
Installing %s...
%s installed!
%s installation failed
In all cases,
%s
stands for Plugin/Theme name and version. To fix the problem with different declensions, we'd need to split these up into 6 different strings and add different comments/contexts for each. If that's not possible, the alternative translation you suggest is probably the best approach.