Rework the translation UI

kkaefer - August 29, 2009 - 15:07
Project:Localization server
Version:6.x-1.x-dev
Component:User interface
Category:feature request
Priority:normal
Assigned:Unassigned
Status:postponed
Description

We need a way to show differences between translations so that changes are more apparent for reviewers, something like:

Attached is a word based diff function I wrote for the German translation server. I'm not sure whether the original code (I wrote it over a year ago) was based on something else, but I think I just used the sample algorithm from http://en.wikipedia.org/wiki/Longest_common_subsequence_problem.

AttachmentSize
diff.algorithm.php.txt4.53 KB
diff.php.txt1.11 KB

#1

kkaefer - August 29, 2009 - 15:14

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

#2

Psicomante - August 29, 2009 - 17:04

cool, +1 for this patch. See what Gàbor think about it ;)

#3

kkaefer - August 29, 2009 - 17:58

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.

AttachmentSize
diff.algorithm.php.txt 5.12 KB

#4

kkaefer - August 29, 2009 - 19:16
Status:active» needs review

Here's a first attempt at a patch.

AttachmentSize
diff.patch 10.09 KB

#5

hass - September 1, 2009 - 18:02

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

#6

Xano - September 4, 2009 - 10:00

I am working on l10n integration for http:/drupal.org/project/atr, which does something similar. It might be cool to put this code in there, so it can be used by other projects as well. That way we centralise all text review methods for Drupal.

#7

kkaefer - September 26, 2009 - 18:36
Title:Show differences between translations» Rework the translation UI

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

AttachmentSize
b59ca531.patch 176.05 KB

#8

kkaefer - September 26, 2009 - 18:37

And here are the images.

AttachmentSize
images.zip 4.44 KB

#9

kkaefer - September 26, 2009 - 18:40

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

#10

kkaefer - September 26, 2009 - 18:41

What’s still missing:

  • Support for contexts
  • Tests (but we probably can’t test the UI)
  • Remove leftovers/code that isn’t being used anymore

#11

Xano - September 28, 2009 - 17:43

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.

#12

Xano - September 27, 2009 - 14:41
Status:needs review» needs work

#13

Gábor Hojtsy - September 28, 2009 - 14:31

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.

AttachmentSize
UIQuickReview.png 119.68 KB

#14

Gábor Hojtsy - September 29, 2009 - 07:53

More thinking / review:

- While this UI is (IMHO) way better then the previous translation tab UI, it does not replace the defining functionality of the moderate screen. When people import big amounts of bad translations, or a different language version, then the moderate screen lets people mass-decline those strings instead of going through them one by one when multiple suggestions exist for the parent strings at hand.

- I'm seeing stabilized strings in the constants but not on the UI.

- Moving selected strings is inconsistent in how it marks declined strings. When you pick a different string, the string picked before has a line-through, but not the others, even though you actually decline that one as well by picking a different string. When the texts become crossed-through, it gets much harder to review the diffs and make corrections in my selection.

PICTURES:

When picking a different translations, only the previously selected one is crossed-though:

When texts actually became crossed-through, it is hard to view changes:

AttachmentSize
MoreUIFeedback-1.png 43.57 KB
TryingToReview.png 30.79 KB

#15

kkaefer - September 29, 2009 - 08:28

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

#16

hass - September 29, 2009 - 10:38

This looks all very promising for usability... do you plan to keep all suggestions in the UI? I remember of 20 or more suggestions for one string... I believe the counter will go up much more on d.o - nevertheless the gold crown gives is a chance to block, we may have 50 suggestions until something becomes golden :-). Not sure if we are able to decline all others in such a case automatically or manually.

#17

Gábor Hojtsy - September 29, 2009 - 11:07

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

#18

kkaefer - October 1, 2009 - 13:27

It is still easy to decline suggestions: You can either click the "Decline" button next to each string or double-click on the string you want to make active. If you double-click, all others are declined. If you select another active translation, the previously active translation is marked as declined. However, you undecline it as well. In that case, the previously active string will become a regular suggestion again.

#19

hass - October 1, 2009 - 13:39

#20

Gábor Hojtsy - October 1, 2009 - 13:42

I was planning to understand the workflow suggestions better here and make up a video of the suggestions before posting for the larger audience on localize.drupal.org, so people who can obviously not install all the modules to test the new UI can still see how it works. I still plan to do that but many other important stuff got into the way, so fixed other issues this week (news post at http://localize.drupal.org/node/214 ).

#21

Gábor Hojtsy - October 1, 2009 - 14:32

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.

#22

kkaefer - October 2, 2009 - 11:19

Restored the moderate button and added a button to copy the source string to the textarea.

AttachmentSize
0f366c24.patch 182.12 KB

#23

kkaefer - October 2, 2009 - 11:21

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

AttachmentSize
cc45f404.patch 170.62 KB

#24

Gábor Hojtsy - October 7, 2009 - 12:23

Did a quick video and posted feedback request at http://localize.drupal.org/node/258

#25

Gábor Hojtsy - October 7, 2009 - 14:15

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.

#26

seutje - October 7, 2009 - 15:02

subscribe

@kkaefer: as I told u in IRC (or on twitter, don't remember) a month ago or so, I can't think of anything I would like to see different in this
that doesn't mean it is perfect as it is and that it is invulnerable to improvement or anything, it just means that the improvement this would provide as it is right now, blows my mind far too hard to be able to perform any sort of proper evaluation

in short: it's sheer awesomeness clouds my judgment

#27

Pasqualle - October 7, 2009 - 15:24

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

#28

toemaz - October 16, 2009 - 13:28

Terrific improvement and thanks for the screencast at localize.drupal.org which made me land here. However, I'm not enough into the translation server stuff yet in order to give some decent feedback. Will give it a try.

#29

toemaz - October 16, 2009 - 14:36

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.

#30

podarok - October 17, 2009 - 08:31

subscribe
Like Your work!

#605748: Approve Selected + Decline unselected at once in moderation can make Us happy too 8)

#31

claudiu.cristea - October 22, 2009 - 09:18

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: Branching a translation 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 (!?)

#32

Gábor Hojtsy - October 22, 2009 - 09:27

The issue at #610898: Unable to import translation on http://localize.drupal.org/ notes that not listing suggestions on the browse/translate UI is confusing. This patch helps clean up that confusion as well :)

#33

hass - October 22, 2009 - 13:28

#608288: Let Google suggest translations is really the most worst idea I have ever heard... 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.

#34

Gábor Hojtsy - October 22, 2009 - 13:30

@hass: can you please pick the right issue to post your followup?

#35

claudiu.cristea - October 22, 2009 - 13:44

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

#36

hass - October 22, 2009 - 14:42

Sry, commented over there...

#37

joachim - November 9, 2009 - 09:00

+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!

#38

Gábor Hojtsy - November 10, 2009 - 09:18
Status:needs work» postponed

The permissions part of this patch was broken out to #220119: Permission to "approve other suggestions only" thanks to @kkaefer for sending a partial patch with only those changes. Postponing on that.

 
 

Drupal is a registered trademark of Dries Buytaert.