Background:
This issue is part of the task to update the hook_help texts of the Drupal 8 modules:
#1908570: [meta] Update or create hook_help() texts for D8 core modules

Tasks:
a) review / write the hook_help text according to help guidelines

b) (novice) Final manual testing:
1. Apply the patch.
2. Go to admin/help.
3. Click on the help page for this module.
4. Verify that the help page is OK:
- Verify that all the links work
- Verify that all mentions of pages/text/permissions within the UI match what is seen in the UI
- Verify that the formatting is OK.

Files: 
CommentFileSizeAuthor
#56 locale-case-help-text-2091459-56.patch8.25 KBfran seva
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 71,753 pass(es).
[ View ]
#56 interdiff_2091459-54-56.txt3.17 KBfran seva
#54 locale-case-help-text-2091459-54.patch8.25 KBfran seva
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 71,728 pass(es).
[ View ]
#54 interdiff_2091459-52-54.txt4.38 KBfran seva
#52 locale-case-help-text-2091459-52.patch6.02 KBfran seva
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 71,752 pass(es).
[ View ]
#52 interdiff_2091459-50-52.txt4.15 KBfran seva
#50 interdiff_2091459-42-50.txt3.71 KBfran seva
#50 locale-case-help-text-2091459-50.patch4.19 KBfran seva
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 71,705 pass(es).
[ View ]
#42 locale-case-help-text-2091459-42.patch4.06 KBfran seva
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 71,645 pass(es).
[ View ]
#42 interdiff_2091459_33-43.txt5.91 KBfran seva
#33 locale-case-help-text-2091459-33.patch7.97 KBfran seva
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 70,688 pass(es).
[ View ]
#27 locale-case-help-text-2091459-27.patch4.07 KBifrik
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 67,061 pass(es).
[ View ]
#27 interdiff-23-17.txt1.71 KBifrik
#23 locale-case-help-text-2091459-23.patch4.07 KBifrik
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,858 pass(es).
[ View ]
#17 locale-help-text-2091459-17.patch5.79 KBifrik
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,868 pass(es).
[ View ]
#17 interdiff-15-17.txt3.16 KBifrik
#15 locale-help-text-2091459-15.patch5.82 KBvisabhishek
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,810 pass(es).
[ View ]
#15 interdiff-2091459-12-15.txt2.56 KBvisabhishek
#13 locale-help-text-2091459-12.patch5.82 KBifrik
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,805 pass(es).
[ View ]
#13 interdiff-6-12.txt4.18 KBifrik
#8 drupal.update-hook-help-for-locale-module.2091459-08.patch5.73 KBvisabhishek
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,786 pass(es).
[ View ]
#6 locale-help-text-2091459-6.patch5.74 KBifrik
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,726 pass(es).
[ View ]
#6 interdiff-2103039-2-6.txt6.3 KBifrik
#2 locale-help-text-2091459-2.patch6.06 KBifrik
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,733 pass(es).
[ View ]

Comments

ifrik’s picture

Issue summary:View changes

working on it during DevDays

ifrik’s picture

Status:Active» Needs review
StatusFileSize
new6.06 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,733 pass(es).
[ View ]

This is an initial re-write of the help text for the Locale module (called Interface Translation in the admin interface).
Just as with the language module, I'm not sure whether I've found all functionality that is provided by the module now.

jhodgdon’s picture

Status:Needs review» Needs work

Thanks! Looks like a good start. A few comments:

a)

The Interface Translation module allows you to translate interface text in different languages
...
how much of the interface on your site is translated in which language

In English, we should say "translate(d)... to" or "translate(d)... into", not "translate(d)... in". [Prepositions are so complicated! It would be so nice if there was a definite correspondence between languages' prepositions... I'm always getting them wrong in Spanish, even though I'm pretty fluent. Anyway.]

b)

http://drupal.org/documentation/modules/locale/
http://localize.drupal.org

All URLs on *.drupal.org should be https.

d)

...or download the currently used translation file ...

currently-used should be hyphenated.

e)

+      $output .= '<dt>' . t('Detecting and selecting the interface languages ') . '</dt>';

I think I would take "the" out of this heading or make "languages" singular. But see below... might just want to remove this section.

f)

On the <a href=!detection">Detection and Selection</a> page, ...

"Selection" should not be capitalized. But see below... might just want to remove this section.

g) I do not think that the language switcher block is provided by the locale module. As far as I can see, the functionality provided by this module is:

1. Settings page (route: locale.settings) - set how often to check for updates, import behavior, and source for translations. This is covered in the help patch.

2. Reports:
- Check translations report (route: locale.check_translation) /admin/reports/translations/check
- Update status report (locale.translate_status) /admin/reports/translations

I don't see these being covered in the help, and I am not sure what they are for really.

3. UI stuff:
- Translate page (route: locale.translate_page)
- Import/export (locale.translate_import, locale.translate_export)

These are covered in your help patch.

===> So, there are a couple of things to add to the help, and I think a couple of things to remove. I would actually suggest putting some text in the About section saying that making a non-English or multilingual site will require some other modules as well, and point them to the Language module help for more information (and other modules? Like Content Translation?), and then omit anything further down that is not a function directly of this module (like the language switcher block and the detection and selection settings).

batigolix’s picture

I updated the docs on do with the new module name. Note the new alias
https://drupal.org/documentation/modules/interface_translation
This needs updating in the hook help text as well

jhodgdon’s picture

Ummmm... For other modules, I think we have used the module's short/machine name for the page alias. So I think the right URL should still be modules/locale ?

ifrik’s picture

Status:Needs work» Needs review
StatusFileSize
new6.3 KB
new5.74 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,726 pass(es).
[ View ]

Thanks, I think I've taken all of them up.
Looks like I had overlooked the language switcher when I did the language module. I've refered back to the language module in the About section now.

jhodgdon’s picture

Status:Needs review» Needs work
Issue tags:+Needs manual testing, +Novice

Looks great! Just one more very small thing:

The Interface Translation module uses functionalities provided by the <a href="!language">Language module</a>

I think I would just say "functionality" here rather than "functionalities". My spell checker doesn't complain about "functionalities", but I don't really think functionality is a noun that should be made plural.

Once that small thing is fixed (or while it is being fixed), this patch is ready for a manual test:
- Verify that all the links work
- Verify that all mentions of pages/text/permissions within the UI match what is seen in the UI
- Verify that the formatting is OK.

visabhishek’s picture

Status:Needs work» Needs review
StatusFileSize
new5.73 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,786 pass(es).
[ View ]

"functionalities" is changed to "functionality" as per #7.

batigolix’s picture

#5 -> oops, yeh. i saw the example of the picture -> responsive images module but that module had actually its short module name changed ...

anyhow, i reversed the alias to documentation/modules/locale

ifrik’s picture

Status:Needs review» Needs work

Thanks for that. I might need to do a few more changes in the next days, after working out with the Multilingual sprint people at the DevDays on how best to bring the interface text and titles in line with the style guide https://drupal.org/node/604342

batigolix’s picture

I verified the patch as requested in #7

All is ok:
- all the links work
- all mentions of pages/text/permissions within the UI match what is seen in the UI
- the formatting is OK.

I found one small thing, though:

You can translate individual text elements (strings)

The term "string" is introduced in the last sentence of the text. I would try to do this in the introduction

ifrik’s picture

Thanks, I take that up.

ifrik’s picture

Status:Needs work» Needs review
StatusFileSize
new4.18 KB
new5.82 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,805 pass(es).
[ View ]

Added the comments from #7 (already taken up in #8) and #13, as well as clarifying that downloading files from the Drupal translation server is optional.
As mentioned before: the page and tab titles should probably be changed, in which case they need to be changed in this help text as well.

visabhishek: I've redone your change to easily make it one interdiff.

jhodgdon’s picture

Status:Needs review» Needs work

Looks good to me! There is some extra indentation on this line near the end:

+            $output .= '<dt>' . t('Translating individual text elements') . '</dt>';

batigolix already did the manual test, so if we can get this fixed, we can mark it RTBC. Thanks!

visabhishek’s picture

Status:Needs work» Needs review
StatusFileSize
new2.56 KB
new5.82 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,810 pass(es).
[ View ]

extra indentation is removed now..

batigolix’s picture

Status:Needs review» Needs work

One more small thing:
Now that the term "strings" is introduced in the first sentence it can be used elsewhere in the text without explanation.

So here:

Translating individual text elements
You can translate individual text elements (strings) ...

I would say:

Translating individual strings
You can translate individual strings ...
ifrik’s picture

Status:Needs work» Needs review
StatusFileSize
new3.16 KB
new5.79 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,868 pass(es).
[ View ]

Good idea.

batigolix’s picture

Status:Needs review» Reviewed & tested by the community

Happiness

jhodgdon’s picture

Status:Reviewed & tested by the community» Fixed

Thanks all! Committed to 8.x.

  • Commit 28e045f on 8.x by jhodgdon:
    Issue #2091459 by ifrik, visabhishek, batigolix: Fix hook_help for...
jhodgdon’s picture

Status:Fixed» Needs work

Actually though... Can we have a quick follow-up to fix the rest of hook_help() -- all of the case ... statements in there are still using the old url() functions and may also need some work?

batigolix’s picture

Component:documentation» locale.module

I am also realizing that we did not get any feedback from the maintainers. I'm pretty sure they would like to give these texts a quick review. So I am changing the component and will try to find somebody AFK here in Szeged to have a look at the patch

ifrik’s picture

Status:Needs work» Needs review
StatusFileSize
new4.07 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,858 pass(es).
[ View ]

Fixed all the links in the cases.

jhodgdon’s picture

Status:Needs review» Needs work

Note on #22 - the earlier patch was already committed, but we can always make a new patch if more changes are needed based on maintainer feedback.

Patch looks good at first glance... probably needs someone to do a manual test and make sure the links in the help at tops of those pages work?

OH, could we fix the grammar and puncutation here (that is on admin/config/regional/translate):

(Note: Because translation tasks involves many strings, it may be more convenient to <a href="!export">export</a> strings for offline editing in a desktop Gettext translation editor).

- involves => involve
- The . should be inside the ) at the end

Thanks!

ifrik’s picture

I didn't do any changes to that UI text, because that will be a bigger task for the four multilingual modules together. can we postpone that for now?

jhodgdon’s picture

Yes, agreed we should focus here on the hook_help() only.

ifrik’s picture

Status:Needs work» Needs review
StatusFileSize
new1.71 KB
new4.07 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 67,061 pass(es).
[ View ]

but since we've seen that one now... here is that typo fixed as well.

batigolix’s picture

I propose to use a common About section for all multilingual modules (#2091479: Update hook_help for content_translation module #30):

So the one for this module would be:

The Interface Translation module allows you to translate interface text (<em>strings</em>) into different languages, and to switch between them for the display of interface text. Together with the modules <a href="[LINK]">Language</a>, <a href="!config-trans">Configuration Translation</a>, and <a href="[LINK]">Content Translation</a>, it allows you to build multilingual websites. For more information, see .....
ifrik’s picture

I like such a common text.

jhodgdon’s picture

OK, should we add #28 to this follow-up patch then? The rest looks fine.

Gábor Hojtsy’s picture

Issue tags:+D8MI, +language-ui

Yeah #28 looks fine. This is used in the config translation docs patch too.

jhodgdon’s picture

We just had a change to hook_help, on this issue: #2183113: Update hook_help signature to use route_name instead of path.

Here is the change record: https://drupal.org/node/2250345

This patch will need a reroll for this change.

fran seva’s picture

StatusFileSize
new7.97 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 70,688 pass(es).
[ View ]

Attach the Re-rolling patch and change the use of Drupal word following the instruction from issue [1]

[1] https://drupal.org/node/2103039#comment-8759659

jhodgdon’s picture

Status:Needs review» Needs work

Thanks!

In this one case, however: <a href="!server">Drupal translation server</a> -- that is actually specifically linking to localize.drupal.org, so it is actually the "Drupal translation server". So we should not take out the word Drupal there. This phrase/link occurs in a couple of places in the patch, and should be restored to using "Drupal".

If you are making a new patch, please also see comment #28 above.

fran seva’s picture

Sorry @jhodgdon. When I was changing the "Drupal translation server" I thought about what you are telling me, but I didn't ask, and I should.
I'm going to read the comment #28 and revert the changes related with word Drupal that shouldn't be taken out.
Thanks for review.

Gábor Hojtsy’s picture

@jhodgdon: in fact the direction on our very active prior issue in #1861930: Use "Drupal translations website" instead of Drupal translation server or Community translation server is that Drupal is NOT to be mentioned in this text either, so to be distribution independent. While localize.drupal.org is the Drupal translation server it is also translation server to most other distributions including Commerce, COD, etc. Best to be distribution independent IMHO. In fact I hoped feedback from the docs team on #1861930: Use "Drupal translations website" instead of Drupal translation server or Community translation server which is why I tagged it with user interface text. Seems like that was not the right magic word :D

jhodgdon’s picture

Thanks for the pointer! Commented on other issue... guess we should wait before deciding what to do here.

fran seva’s picture

@gaborhojtsy @jhodgdon I'm going to wait till the decision before continue with the patch and tests.

jhodgdon’s picture

@fran seva: The other option would be to take these changes out of the patch, and just do the other changes.

fran seva’s picture

@jhodgdon oki. It's a good option :)

fran seva’s picture

Assigned:Unassigned» fran seva
Issue tags:+DrupalCampSpain
fran seva’s picture

StatusFileSize
new5.91 KB
new4.06 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 71,645 pass(es).
[ View ]

Attach the patch and interdiff without the changes related with "Drupal translation server".
I'll be checking the issue #2091459. I think I shouldn't start with the manual test until we known what to do with the string "Drupal translation server" but if you think it's necessary just tell me and I'll do it :)

fran seva’s picture

Status:Needs work» Needs review
jhodgdon’s picture

I suppose we should just wait for the other issue to get resolved now.

Gábor Hojtsy’s picture

@jhodgdon: I think this fixes so many other things and adds more value to the UI even until the Drupal translation server discussion happens (if ever). So it would be great to get this in soon IMHO. Any remaining concerns with the current patch?

jhodgdon’s picture

Status:Needs review» Needs work

I took a look at the latest patch and I think it's mostly OK.

A few concerns:

a) language.admin_overview

Interface text can be <a href="!translate">customized</a>

This link text is not Accessible. It either needs a link title attribute added or better link text (the principle is that link text should describe where the link is going, and "customized" does not do this at all).

b) locale.translate_import

downloaded and imported when <a href="!language">languages</a> are added

This link is marginally OK since it points to the language overview page, but again it could use a link title for better accessibility.

c) later in that same cas:

<a href="!export">export</a> translations from the site

Again, non-accessible link text.

fran seva’s picture

@jhodgdon I'm on it.
Thanks for review.

fran seva’s picture

@jhodgdon that's my proposal:

a) language.admin_overview
As you said the link text is not describing the where the link is going, so the text could be:

Interface text can be customized in the <a ttitle="User interface translation" href="!translate">user interface translation</a> page

The other option is just add the title attribute with the target page title of the link.

Interface text can be <a title="User interface translation" "href="!translate">customized</a>

I think the better option is the first one because we satisfy accessibility and we got a descriptive title link

b) OK

c) In this case the page title is "Export" but I think that could be better if we
use "User interface translation export"

<a title="User interface translation export" href="!export">export</a> translations from the site

Also, I see that there is no link with title attribute and if we are saying that cases a) and b)
why don't we add it for all links to satisfy accessibility?

jhodgdon’s picture

RE #48 -- In your first option, you do not need a "title" attribute for the link, because the link text is the same as the title, so that can be left out. I like the wording of your first option better too.

So... Just to be clear: Accessibility guidelines require that (preferably) the link text describe where the link is going. If that is not true, then you must instead provide a link title. But providing a link title that is the same as the link text is not necessary or desirable. So:

... go to the <a href="whatever">foo bar page</a> to configure your foo ...

is perfectly fine for accessibility, without a title, because "foo bar page" is where the link is going. But:

... <a href="whatever">configure</a> your foo ...

is not OK because "configure" does not describe where the link is going.

Does that make sense? So please do not add link titles to links that already have descriptive link text. Changing link text so it is descriptive is the best option, and if that is not possible, then at least we need a descriptive link title.

fran seva’s picture

Status:Needs work» Needs review
StatusFileSize
new4.19 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 71,705 pass(es).
[ View ]
new3.71 KB

@jhodgdon thanks for your comment, I tried to found an accessibility doc to be sure about how should be use the title attribute but I didn't found it. Now I know how must be.

Attach the patch and the interdiff.

jhodgdon’s picture

Status:Needs review» Needs work

Hm... This still needs some work:

<a title="User interface translation" href="!translate">user interface translation</a>

This is an example where you do *not* need a title, because the link text "user interface translation" is the same as the title attribute (aside from capitalization, which is fine). Please remove the title attribute.

Please check the other links; some of them have the same problem.

This is a good example in your patch:

<a title="User interface translation export" href="!export">export</a>

Elsewhere in the patch, that same "export" page is missing the link title.

fran seva’s picture

Status:Needs work» Needs review
StatusFileSize
new4.15 KB
new6.02 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 71,752 pass(es).
[ View ]

@jhodgdon thanks again for reviewing the patch again.

I have removed the title attribute from: <a title="User interface translation" href="!translate">user interface translation</a>
and I have add the title attribute in the rest !export links. In this point, I was doubtful in this link
file for a specific language on the <a title="User interface translation export" href="!export">Export</a> page. because I thought that "Export page" was contextualized and a user may know that Export page is referring to the Export translation admin page. Anyway, I decided add the title attribute.

Attach a new patch and interdiff.

jhodgdon’s picture

Status:Needs review» Needs work

The whole point of link accessibility is that there is not necessarily any context. See http://www.w3.org/TR/UNDERSTANDING-WCAG20/navigation-mechanisms-refs.html ... well I guess they do consider context, but it is still preferable to have link text or a title that explicitly tells you where you are going.

So I think we should add a title to <a href="!import">Import</a> like you did for Export.

The rest of the changes look good!

fran seva’s picture

Status:Needs work» Needs review
StatusFileSize
new4.38 KB
new8.25 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 71,728 pass(es).
[ View ]

Attach the patch and the Interdiff.

jhodgdon’s picture

Issue summary:View changes

Great! I think this one is ready for a quick manual test:
- Verify that all the links work
- Verify that all mentions of pages/text/permissions within the UI match what is seen in the UI
- Verify that the formatting is OK.

fran seva’s picture

StatusFileSize
new3.17 KB
new8.25 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 71,753 pass(es).
[ View ]

I have detected an HTML bug in the first Setting link:
<a href=!locale-settings">Settings</a>

Should be
"!locale-settings">Settings

Attach the patch and the interdiff

After apply the patch I have tested the help page:

- Verify that all the links work

All links are working

- Verify that all mentions of pages/text/permissions within the UI match what is seen in the UI

OK

- Verify that the formatting is OK.
The formatting is OK

jhodgdon’s picture

Status:Needs review» Reviewed & tested by the community

Good catch, and thanks for testing!

fran seva’s picture

Assigned:fran seva» Unassigned
jhodgdon’s picture

Status:Reviewed & tested by the community» Fixed

Thanks again everyone! Committed to 8.x.

Gábor Hojtsy’s picture

Yay, thanks all!

develCuy’s picture

Issue tags:-Needs manual testing
fran seva’s picture

@develcuy I did a manual test in #56. Is necessary do it again?

jhodgdon’s picture

Fran: This issue has been fixed. It is done. Thanks!

fran seva’s picture

@jhodgdon: oki :)

  • Commit f1b3b88 on 8.x by jhodgdon:
    Issue #2091459 by fran seva, batigolix, ifrik, visabhishek: Update...

Status:Fixed» Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.