Rework the translation UI

Note: we have to call setlocale before generating diffs. Without that, the regular expression won’t recognize non-word symbols properly.

Won't split Unicode characters now (using mbstring directly until a patch for proper inclusion into Drupal is ready) and improved detection of prefixes from differing subsequences.

Cool... looks much better to me with the colors than the strike through variant 1 year ago...

Integrated the new UI and threw out the Edit, View and Moderate tabs. They are now all in one UI.

Demo of the UI is at http://kkaefer.com/l10n/index.html

With strings that have the same word twice, like "Der Pfadpräfix oder die Sprachdomain" versus "Der Pfadpräfix oder die die Sprachdomain", the second "die" doesn't get displayed in the results.

I only tested the patch with the PHP algorithm. According to kkaefer the JS verison doesn't have this problem.

Here is a very quick review

1. Code review tells me there are code style problems in the patch. Eg if() statements without {}
2. Code review tells me you add a full on jQuery 1.3 into the module. Why not depend on jQuery update module?
3. I did not look at query builder changes but I really wonder what "contains" would search in. It should search in any string that is still active and displayed. Looks like it does search in suggestions.
4. I did run some pretty basic tests of the UI and noticed a few issues:

My big picture of this patch, is that its functionality looks amazingly cool. It does a frightening amount of changes introducing new permissions, the diff functionality as well as getting rid of two separate screens, etc. To make sure that all this goes flawlessly, we also need the included automated tests updated, so we can ensure all works as expected. There is very good test coverage for the suggestions / translations / filters, etc.

I'd love to do a quick screencast of how it works and what would be my expectations, so we can get some usability feedback quickly.

A couple of answers:

• The duplicate info was there by default; I just forgot to remove it for this patch. Noted.
• “I cannot suggest, only translate”: By default, all strings are suggestions. To mark a string as accepted (“translated”), you have to check the radio button since there can only be one active translation.
• I didn’t change anything about the filter toolbar and therefore didn’t modify the way the “contains” field works.
• I’m okay with adding the “Moderate” screen back in.
• The “stabilized string” still needs to be added (I have code for that, but it’s not fully integrated yet, therefore it’s commented out).
• The fact that other strings aren’t automatically declined is intentional. The reason for this is that often times, you don’t want to read all suggestions (especially with long strings) and it’s easier to be able to decline strings individually or mark a string as active while reading through the list. This also means that you can use the controls and don’t have to remember which string you liked best so far. If you still want to decline all other strings, you can double click on the string you want to mark as active.
• If you want to view the changes after you already declined a string, you can click on the undecline button, review the changes and then declined it again (or mark it as active).

This system still makes it possible to decline suggestions (which was not too intuitive for me judging from the explanation from Konstantin above). The outstanding suggestions are displayed with the active translation. (Also, previously approved but not anymore active translations are not shown either).

Hah, I've actually found via twitter that you have a video at http://i.kkaefer.com/l10n.mov Which shows some of the features I've not even realized you have added :) It also underlines one removed/missing feature through: you cannot copy the source string to quickly start a translation. That would quite good in places where the source has HTML code or many placeholders as shown in your examples. Also, sometimes it is hard to make out what you mean as there is no sound on the video.

Oops, here's the patch with moderate.inc not removed.

My quick feedback is as follows:

- still don't know why is l10n_community not dependent on http://drupal.org/project/jquery_update then, instead of including it in the module itself
- managed to understand while testing is that you always add a suggestion (and the feedback in the dsm is misleading in saying "translation"!), even if you are an admin, you need to enter the suggestion *and* pick that to make it appear as the active one (now that I understand it sounds like a logical workflow, but the feedback on submit would be better fixed)
- the marking as stable is not "missing" per say, it has some implementation items, like "mark other people's translations as stable" appearing int he l10n_community_get_permission() function
- noticed that not all placeholders are properly detected, while @url is fine, @drupal-link is not (not sure it was this exact placeholder, but noticed - is treated as a separator, but it is not).

All-in-all, I'm happy with the functionality of the patch. Do you see any possibility of staging its inclusion somehow, or would that be more work then how useful it is? Eg. I'd imagine permission changes can be committed first, but then at a loss of ideas on how could we divide the changes. Reviewing smaller chunks would obviously be easier.

Also, underlining that adapting tests to the changes is mandatory. I'll try to do that later if you do not have resources to work on that.

I would prefer only one "more information" link on the page, and that would show/hide the usage info for all strings. Because when I need that info, then I have to click many of these links to find the correct string I want to translate..

Great improvements..

Did anyone recently apply the #23 patch against the latest 6.1 branch? I'm having a hard time applying it with eclipse so if anyone has a working version, it would be great if one could share a new patch. I will then apply it on a live server (which is currently up for testing) so anybody can give it a try

Another note: I concur to reuse jquery_update as noted by Gábor in #25.

This is great!

The UI should have also support for the below features. Some of them are not subject of this ticket but are listed here because of their impact on UI layout...

1. A UI button/icon for creating/overwriting suggestion from source string.
2. A UI button/icon for creating/overwriting suggestion from Google Translate. #608288: Let Google suggest translations is dealing with this but i recall here because of its impact on UI.
3. Branch support. This is now only a proposal in #608488: Translation inheritance but if implemented, then each suggestion must be linked to a branch (defaulting to base) and we must have the ability to setup this through UI. Add here also for UI geometry/layout reason. Problem: This mean that we can have more than one translated item but no more than one per branch (!?)

#608288: Let Google suggest translations is really the most worst idea I have ever read about... I would never use Google as translation service. Have you ever read the damn ugly sh** that came out? I believe not... better translate by hand than using Google translation service.

EDIT: Changed "heard" to "read about" so that no one reads "had"

@hass

... Let Google suggest translations is really the most worst idea I have ever heard ....

It's about suggesting not about definitive translations... It's a little bit more than "copy from source". You don't have to use it if you don't like it...

+1 for general reworking.

But scrolling through this issue, I see the screenshot, and I'd like to say -- little fiddly icons beyond the obvious small set of '+' and 'X' and '?' are hard to understand!
On the UI as it currently stands there is a sort of <-> icon and I have NO IDEA what it means!

#220119: Permission to "approve other suggestions only" is now committed, so code related to that should be removed from this patch. This unblocks this issue, so people can keep working on it. To actually be able to deploy #220119: Permission to "approve other suggestions only" in a meaningful way on l.d.o, we still need some more changes, but those are orthogonal to a reworked translation UI. Opened an issue for that with an initial patch at #652038: Migrate permissions to og_user_roles, instead of our own code.

@kkaefer: can you pick up working on this with a patch that does not contain the permission changes anymore? :)

@kkaefer: The #40 patch have errors. Eclipse complains about l10n_community.js and ajax.inc. Patch shows they should be deleted... no idea why Eclipse complains. I fixed the patch and removed jquery13.js and added the dependency with jquery_update.

@gabor: Do we need to check with hook_requirement if jquery_update 6.x-2.x is installed or are you fine with the jquery_update dependency only that may also allow to install jquery_update 6.x-1.x?

• Use jQuery Update.module

Replaced jQ13 with jQuery

I've added the images from #8 to CVS so patch testing should be easier. http://drupal.org/cvs?commit=310236

Tried to foster some more interest for helping out with this at http://localize.drupal.org/node/708 a week ago but did not seem to succeed.

I sincerely hope we can deploy this *way* before Drupalcon SF. (Am I dreaming? :)

This fallen out of date a bit. Trying to fix hunks so it applies again:

So this is the meat of the patch that was not applied yet and still needs to be fixed to be able to apply / commit. Working on it.

Ok, integrated the rest of the patch and committed as well. Looks to be working well on DRUPAL-6--2.

We still need to:

- clean up the code, add comments, etc.
- verify how it works in different permission scenarios (it replaces both the view and edit UI for people)
- align tests, so they work well with the UI as it is in place; they are breaking badly ATM

Great! THX!!!

Thanks everyone - this is one awesome improvement!

Reviewing the translate page I repeatedly get the JS error: "$("em.l10n-placeholder").live is not a function". Although referenced in css I couldn't find any place where a DOM object is created with that id.

After removing that section from editor.js I got the following JS error: "translation.mouseenter is not a function"

Situation:
Firefox 3.5.7 on Ubuntu 9.10
Logged in as user 1
One project added with the l10n_localpacks connector

Otherwise ans awesome improvement.

I took the opportunity to do some minor cleanup, partly suggested by the coder module, see patch.

@Gábor Hojtsy: I had 6.x-1.1 of the jquery_update module.
Just updated to 2.0-dev and that solved it.

From #57:

- clean up the code, add comments, etc.

Now mostly done, I think I'm confident enough in the code now (minus the worddiff algorithm internals I did not take time with, but not worries about :).

- verify how it works in different permission scenarios (it replaces both the view and edit UI for people)

Only minimally done yet. Still needs to be done.

- align tests, so they work well with the UI as it is in place; they are breaking badly ATM

I've been working with this over the weekend. Now fully done at #715606: Update test on 6.x-2.x to reflect current reality.

Looks like this needs some manual verification still, but otherwise shaping up well and is well on its way to deployment :)

Worked a bit on making the "more information" feature more useful. Removed "used in" and renamed link to "show occurrences". Made it possible to be hidden. This is committed over at #715984: Restore ability to hide "more info".

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