Update hook_help for Locale module

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).

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

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 ?

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.

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

#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

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

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!

extra indentation is removed now..

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 ...

Happiness

Thanks all! Committed to 8.x.

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?

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

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!

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

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 .....

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

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

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.

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

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.

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.

@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

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

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

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

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

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 :)

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

@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?

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.

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

@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?

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.

@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.

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.

@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.

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!

Attach the patch and the Interdiff.

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.

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

Good catch, and thanks for testing!

Thanks again everyone! Committed to 8.x.

Yay, thanks all!

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

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

@jhodgdon: oki :)