Skip to content

MarkUnknown checkbox visible with TranslateDoc - Fix #146 #160

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

21 changes: 13 additions & 8 deletions assets/js/translator.js
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,9 @@ if(modeEnabled('translation')) {
if($('div#translateText').is(':visible')) {
translateText();
}
else if($('div#docTranslation').is(':visible')) {
translateDoc();
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Um, I am not sure why this is appearing in the diff here :/ . I simply did a git rebase with master. Do you suggest making a separate PR? (The below snippet has the changes that you asked to do @sushain97 )!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, maybe you're rebasing with your master instead of origin/master? I think these changes are separate so we don't want to merge them.

Copy link
Contributor Author

@share-with-me share-with-me Aug 2, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Um, I did a git fetch origin, git rebase origin/master. I am not sure why it still exists. Is it okay if I create a new PR?

persistChoices('translator');
});

Expand Down Expand Up @@ -218,12 +221,13 @@ if(modeEnabled('translation')) {
});

$('button#translateDoc').click(function () {
$('div#translateText').fadeOut('fast', function () {
$('#fileInput').show();
$('div#fileName').hide();
$('div#docTranslation').fadeIn('fast');
$('#detect, #srcLangSelect option[value=detect]').prop('disabled', true);
});
$('#translateText > *:not(#translateOptionsContainer), #translateOptions > *:not(#markUnknownContainer')
.fadeOut('fast', function () {
$('#fileInput').show();
$('div#fileName').hide();
$('div#docTranslation').fadeIn('fast');
});

pairs = originalPairs;
populateTranslationList();
});
Expand All @@ -233,7 +237,8 @@ if(modeEnabled('translation')) {
$('div#docTranslation').fadeOut('fast', function () {
$('a#fileDownload').hide();
$('span#uploadError').hide();
$('div#translateText').fadeIn('fast', synchronizeTextareaHeights);
$('#translateText > *:not(#translateOptionsContainer), #translateOptions > *:not(#markUnknownContainer')
.fadeIn('fast', synchronizeTextareaHeights);
$('input#fileInput').wrap('<form>').closest('form')[0].reset();
$('input#fileInput').unwrap();
$('#detect, #srcLangSelect option[value=detect]').prop('disabled', false);
Expand Down Expand Up @@ -568,7 +573,7 @@ function populateTranslationList() {
}

function translate() {
if($('div#translateText').is(':visible')) {
if($('#translateText > *:not(#translateOptionsContainer), #translateOptions > *:not(#markUnknownContainer').is(':visible')) {
translateText();
}
else if($('div#docTranslation').is(':visible')) {
Expand Down
4 changes: 2 additions & 2 deletions index.html.in
Original file line number Diff line number Diff line change
Expand Up @@ -191,9 +191,9 @@
<i class="fa fa-file-text"></i> <span data-text="Translate_Document">Translate a document</span>
</button>
</div>
<div class="col-lg-3 col-md-6 pull-right">
<div class="col-lg-3 col-md-6 pull-right" id="translateOptionsContainer">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, I thought about this a bit and I'm not actually a huge fan of the siblings solution. It's clever and is probably the minimal diff or something but it requires carefully inspecting the HTML to figure out what things are siblings, etc. I would prefer a more explicit approach, e.g.

hiding/showing #translateText *:not(#translateOptionsContainer), #translateOptions *:not(#markUnknownContainer)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure. I would get on with this approach.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

<div class="checkbox" id="translateOptions">
<label class="pull-right">
<label class="pull-right" id="markUnknownContainer">
<input type="checkbox" id="markUnknown" checked>
<span data-text="Mark_Unknown_Words">Mark unknown words</span>
</label>
Expand Down