Skip to content

Commit

Permalink
Bug 1392255: Address bugs found during testing
Browse files Browse the repository at this point in the history 
- Strip trailing newlines for easier translated status detection. This
  fixes the bug that prevented switching back to the rich FTL editor
  after switching to source view for untranslated pluralized source
  strings.

- Prevent unsaved changes warning to show up when saving virtually any
  FTL translation.

- Treat strings containing only text and references to external
  arguments and other messages as simple strings.

- Extend copy from helpers support for FTL strings:
  * Allow copy from history and other locales in source editor
  * Allow copy from other locales in rich FTL editor

- Allow switching to rich FTL if source editor empty.

- Add ability to display custom plural forms. Previously, FTL editor
  only supported pluralized translations that used variants for one of
  the CLDR plural forms available for the locale. FTL also allows adding
  custom variants, usually numbers, usually 0. Because "No strings"
  sounds better than "0 Strings".

- In FTL source editor, use default tab behaviour.
  • Loading branch information
@mathjazz
mathjazz committed Dec 18, 2017
1 parent 0591562 commit 0d5ccc1
Show file tree
Hide file tree
Showing 2 changed files with 124 additions and 78 deletions.
164 changes: 99 additions & 65 deletions pontoon/base/static/js/fluent_interface.js
View file
Expand Up @@ -5,6 +5,24 @@ var Pontoon = (function (my) {
var fluentParser = new FluentSyntax.FluentParser({ withSpans: false });
var fluentSerializer = new FluentSyntax.FluentSerializer();

function renderPluralEditor(value, name, pluralIndex) {
var example = Pontoon.locale.examples[pluralIndex];

$('#ftl-area .main-value ul')
.append(
'<li class="clearfix">' +
'<label class="id built-in" for="ftl-id-' + name + '">' +
'<span>' + name + '</span>' +
(Pontoon.fluent.isCLDRplural(name) && example !== undefined ? '<span> (e.g. ' +
'<span class="stress">' + example + '</span>' +
')</span>' : '') +
'<sub class="fa fa-remove remove" title="Remove"></sub>' +
'</label>' +
Pontoon.fluent.getTextareaElement(name, value) +
'</li>'
);
}

return $.extend(true, my, {
fluent: {

Expand Down Expand Up @@ -56,44 +74,36 @@ var Pontoon = (function (my) {
.show();
}
// Plurals
else if (entity.isFTLplural) {
else if (self.isPlural(entityAST)) {
var pluralForms = Pontoon.locale.cldr_plurals.map(function (item) {
return Pontoon.CLDR_PLURALS[item];
});

var variants;
if (translationAST) {
variants = translationAST.value.elements[0].variants;
}

if (!Pontoon.locale.examples) {
Pontoon.generateLocalePluralExamples();
}

pluralForms.forEach(function (pluralStr, pluralInt) {
var value = '';

if (translationAST) {
for (var i = 0; i < variants.length; i++) {
if (variants[i].key.name === pluralStr) {
value = self.serializePlaceables(variants[i].value.elements);
break;
}
}
}
// Translated
if (translationAST) {
translationAST.value.elements[0].variants.forEach(function (variant, i) {
var value = self.serializePlaceables(variant.value.elements);
var name = variant.key.name || variant.key.value;
var pluralIndex = pluralForms.findIndex(function(item) {
return item === name;
});

renderPluralEditor(value, name, pluralIndex);
});
}
// Untranslated
else {
pluralForms.forEach(function (pluralName, pluralIndex) {
var value = '';
var name = pluralName;

$('#ftl-area .main-value ul')
.append(
'<li class="clearfix">' +
'<label class="id built-in" for="ftl-id-' + pluralStr + '">' +
'<span>' + pluralStr + ' (e.g. </span>' +
'<span class="stress">' + Pontoon.locale.examples[pluralInt] + '</span>)' +
'<sub class="fa fa-remove remove" title="Remove"></sub>' +
'</label>' +
self.getTextareaElement(pluralStr, value) +
'</li>'
);
});
renderPluralEditor(value, name, pluralIndex);
});
}

$('#only-value').parents('li').hide();
}
Expand Down Expand Up @@ -179,13 +189,55 @@ var Pontoon = (function (my) {

/*
* Is ast of a simple string?
*
* A simple string has no attributes or tags,
* and the value can only contains text and
* references to external arguments or other messages.
*/
isSimpleString: function (ast) {
if (
ast &&
!ast.attributes.length &&
!ast.tags.length &&
ast.value &&
ast.value.elements.length === 1 &&
ast.value.elements[0].type === 'TextElement'
ast.value.elements.every(function(item) {
return [
'TextElement',
'ExternalArgument',
'MessageReference'
].indexOf(item.type) >= 0;
})
) {
return true;
}

return false;
},


/*
* Is CLDR plural name?
*/
isCLDRplural: function (name) {
return Pontoon.CLDR_PLURALS.indexOf(name) !== -1;
},


/*
* Is ast of a pluralized string?
*/
isPlural: function (ast) {
var self = this;

if (
ast &&
ast.value &&
ast.value.elements &&
ast.value.elements.length &&
ast.value.elements[0].expression &&
ast.value.elements[0].variants.every(function (element) {
return self.isCLDRplural(element.key.name) || element.key.type === 'NumberExpression';
})
) {
return true;
}
Expand Down Expand Up @@ -327,8 +379,12 @@ var Pontoon = (function (my) {
$('#original').hide();
$('#ftl-original').show();

// Simple string: only value
if (self.isSimpleString(ast)) {
original += '<li><p>' + self.serializePlaceables(ast.value.elements) + '</p></li>';
}
// Plurals
if (entity.isFTLplural) {
else if (self.isPlural(ast)) {
var variants = ast.value.elements[0].variants;
variants.forEach(function (item) {
original += '<li>' +
Expand All @@ -339,14 +395,6 @@ var Pontoon = (function (my) {
});

}
else if (
ast.value &&
ast.value.elements.length === 1 &&
ast.value.elements[0].type === 'TextElement'
) {
original += '<li><p>' + self.serializePlaceables(ast.value.elements) + '</p></li>';
}

// Attributes
if (ast.attributes && ast.attributes.length) {
ast.attributes.forEach(function (attr) {
Expand Down Expand Up @@ -393,7 +441,7 @@ var Pontoon = (function (my) {
).join('');

if (!richTranslation.length) {
translation = '';
translation = entity.key + ' = \n';
}
}

Expand Down Expand Up @@ -423,11 +471,12 @@ var Pontoon = (function (my) {
}
else if (this.isFTLEditorEnabled()) {
// Main value
var entityAST = fluentParser.parseEntry(entity.original);
var value = $('#ftl-area > .main-value textarea').val();
var attributes = '';

// Plurals
if (entity.isFTLplural) {
if (this.isPlural(entityAST)) {
value = '';
var variants = $('#ftl-area .main-value li:visible');
var nonEmptyVariants = [];
Expand All @@ -453,8 +502,7 @@ var Pontoon = (function (my) {
});

if (value) {
var entity_ast = fluentParser.parseEntry(entity.original);
value = '{ $' + entity_ast.value.elements[0].expression.id.name + ' ->' + value + '\n }';
value = '{ $' + entityAST.value.elements[0].expression.id.name + ' ->' + value + '\n }';
}
}

Expand Down Expand Up @@ -536,28 +584,10 @@ var Pontoon = (function (my) {
response = this.serializePlaceables(attributes[0].value.elements);
}
}

// Plurals
if (
ast.value &&
ast.value.elements &&
ast.value.elements.length &&
ast.value.elements[0].expression &&
ast.value.elements[0].variants
) {
if (this.isPlural(ast)) {
var variants = ast.value.elements[0].variants;
var isFTLplural = variants.every(function (element) {
var key = element.key.name;
var isPlural = Pontoon.CLDR_PLURALS.indexOf(key) !== -1;
var isInteger = element.key.type === 'NumberExpression';

return isPlural || isInteger;
});

if (isFTLplural) {
response = this.serializePlaceables(variants[0].value.elements);
entity.isFTLplural = true;
}
response = this.serializePlaceables(variants[0].value.elements);
}
}

Expand Down Expand Up @@ -594,7 +624,11 @@ $(function () {
if ($(this).is('.active')) {
translation = $('#translation').val();

var translated = (translation !== entity.key + ' = ');
// Strip trailing newlines for easier translated status detection
translation = translation.replace(/\n$/, '');

var translated = (translation && translation !== entity.key + ' = ');

var isRichEditorSupported = Pontoon.fluent.renderEditor({
pk: translated, // An indicator that the string is translated
string: translation,
Expand Down
38 changes: 25 additions & 13 deletions pontoon/base/static/js/translate.js
View file
Expand Up @@ -157,7 +157,7 @@ var Pontoon = (function (my) {
self.markPlaceables(translationString) +
'</p>' +
'<p class="translation-clipboard">' +
self.doNotRender(translationString) +
self.doNotRender(this.string) +
'</p>' +
'</li>');

Expand Down Expand Up @@ -308,12 +308,9 @@ var Pontoon = (function (my) {
((i > 0) ? self.diff(baseString, translationString) : self.markPlaceables(translationString)) +
'</p>' +
'<p class="translation-clipboard">' +
self.doNotRender(translationString) +
self.doNotRender(this.string) +
'</p>' +
'</li>');

// Storing string value, needed for FTL
list.find('[data-id="' + this.pk + '"]')[0].string = this.string;
});

$("#helpers .history time").timeago();
Expand Down Expand Up @@ -451,7 +448,7 @@ var Pontoon = (function (my) {
* Update cached translation, needed for unsaved changes check
*/
updateCachedTranslation: function () {
this.cachedTranslation = $('#translation').val();
this.cachedTranslation = this.fluent.getTranslationSource();
},


Expand Down Expand Up @@ -635,7 +632,6 @@ var Pontoon = (function (my) {
$('#translation-length').show().find('.original-length').html(original.length);
self.moveCursorToBeginning();
self.updateCurrentTranslationLength();
self.updateCachedTranslation();

// Update entity list
$("#entitylist .hovered").removeClass('hovered');
Expand All @@ -658,6 +654,7 @@ var Pontoon = (function (my) {

self.fluent.toggleButton();

self.updateCachedTranslation();
self.updateHelpers();
self.pushState();
},
Expand Down Expand Up @@ -1859,6 +1856,21 @@ var Pontoon = (function (my) {
return;
}

// Special case in FTL source editor
if ($('#translation').is(':visible') || $('#ftl').is('.active')) {
e.preventDefault();

var textarea = $('#translation')[0];
var oldStart = textarea.selectionStart;
var start = textarea.value.substring(0, textarea.selectionStart);
var end = textarea.value.substring(textarea.selectionEnd);

textarea.value = start + '\t' + end;
textarea.selectionEnd = oldStart + 1;

return;
}

var section = $('#helpers section:visible'),
index = section.find('li.suggestion.hover').index() + 1;

Expand Down Expand Up @@ -1961,6 +1973,8 @@ var Pontoon = (function (my) {

// Copy helpers result to translation
$('#helpers section').on('click', 'li:not(".disabled")', function (e) {
var source = $(this).find('.translation-clipboard').text();

// Ignore clicks on links and buttons
if ($(e.target).closest('a, menu button').length) {
return;
Expand All @@ -1977,16 +1991,14 @@ var Pontoon = (function (my) {
}

// FTL Editor
if (self.fluent.isFTLEditorEnabled() && $('#helpers .history').is(':visible')) {
if (self.fluent.isFTLEditorEnabled() && !$('#helpers .machinery').is(':visible')) {
self.fluent.renderEditor({
pk: $(this).data('id'),
string: this.string
pk: true,
string: source
});

// Standard Editor
} else {
var source = $(this).find('.translation-clipboard').text();

self.updateAndFocusTranslationEditor(source);
self.moveCursorToBeginning();
self.updateCurrentTranslationLength();
Expand Down Expand Up @@ -2574,7 +2586,7 @@ var Pontoon = (function (my) {
}

var pf = self.getPluralForm(true);
self.cachedTranslation = translation;
self.cachedTranslation = self.fluent.getTranslationSource();
self.updateTranslation(entity, pf, data.translation);
self.updateInPlaceTranslation(data.translation.string);
self.updateFilterUI();
Expand Down

0 comments on commit 0d5ccc1

Please sign in to comment.