Skip to content

Commit

Permalink
New implementation which keeps nested HTML tags
Browse files Browse the repository at this point in the history
The previous implementation pushed with commit 83c7f8f fails the
tests and I believe is wrong.

As far as I can understand .textNodes() returns a list of all
text nodes of the current element, which are then stripped of the
--i18n.key-- annotations.

The trouble is that .textNodes() uses .contents() which only
searches the immediate children in the DOM tree. Thus is we have
nested HTML tags the text nodes of these nested tags are not
returned and the i18n annotations are not removed.
This is indeed what is causing the existing tests to fail:
https://travis-ci.org/railslove/i18n_viz/jobs/158501488

This new implementation doesn't use .textNodes() and instead
recurses into child nodes on its own.
  • Loading branch information
atodorov committed Jun 2, 2017
1 parent 6f2ee9d commit 0f731b8
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 8 deletions.
18 changes: 12 additions & 6 deletions assets/javascripts/i18n_viz.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,13 +40,19 @@ $.fn.enrichWithI18nData = function() {
return $i18n_element;
};

$.fn.clearI18nText = function() {
var $el;
$el = $(this);
$el.textNodes().each(function(index, node) {
node.textContent = node.textContent.replace(I18nViz.global_regex, "");
function clear_i18n(el) {
el.contents().each(function(index, node) {
if (node.nodeType == 3) {
node.textContent = node.textContent.replace(I18nViz.global_regex, "");
} else {
return clear_i18n($(node));
}
});
return $el;
return el;
};

$.fn.clearI18nText = function myself() {
return clear_i18n($(this));
};

$.extend($.expr[':'], {
Expand Down
4 changes: 2 additions & 2 deletions spec/javascripts/i18n_viz_spec.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ describe "extractI18nKeysFromText()", () ->
it "should return i18n key when 1 present", () ->
expect(window.I18nViz.extractI18nKeysFromText("some text --i18n.key--")).toEqual(["i18n.key"])

it "should return null with no keys present", () ->
it "should return all keys present", () ->
expect(window.I18nViz.extractI18nKeysFromText("some text --i18n.key-- more text --another.key-- end")).toEqual(["i18n.key", "another.key"])

it "should return null with no keys present", () ->
Expand All @@ -24,7 +24,7 @@ describe "jQuery extensions", () ->
expect($jquery_element.text()).toEqual("some text ")

it "should keep child HTML tags", () ->
$jquery_element.append($("<span>some more text --i18n.key2--</span>"))
$jquery_element.append("<span>some more text --i18n.key2--</span>")
$jquery_element.clearI18nText()
expect($jquery_element.text()).toEqual("some text some more text ")
expect($jquery_element.has('span')).toBeTruthy()
Expand Down

0 comments on commit 0f731b8

Please sign in to comment.