-
Notifications
You must be signed in to change notification settings - Fork 90
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
Spell Checker Interface [WIP] #253
base: master
Are you sure you want to change the base?
Conversation
assets/js/spellchecker.js
Outdated
content[splitWords[originalWordsIndex]] = '<div class="list-group">'; | ||
for(var sugg = 0; sugg < data[tokenIndex]['sugg'].length; sugg++) { | ||
content[splitWords[originalWordsIndex]] += '<a href="#" class="list-group-item">' + | ||
data[tokenIndex]['sugg'][sugg][0] + '</a>'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
["sugg"] is better written in dot notation dot-notation
assets/js/spellchecker.js
Outdated
$('#spellcheckerInput').html($('#spellcheckerInput').html() + ' <span class="spellError" id=' + | ||
splitWords[originalWordsIndex] + '>' + splitWords[originalWordsIndex] + '</span>'); | ||
content[splitWords[originalWordsIndex]] = '<div class="list-group">'; | ||
for(var sugg = 0; sugg < data[tokenIndex]['sugg'].length; sugg++) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
["sugg"] is better written in dot notation dot-notation
assets/js/spellchecker.js
Outdated
success: function (data) { | ||
var originalWordsIndex = 0; | ||
for(var tokenIndex = 0; tokenIndex < data.length; tokenIndex++) { | ||
if(data[tokenIndex]['known'] === true) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
["known"] is better written in dot notation dot-notation
a2ea8f0
to
7583760
Compare
assets/js/spellchecker.js
Outdated
placement: 'bottom', | ||
trigger: 'manual', | ||
html: true, | ||
content: content[currentTokenId] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'content' is not defined no-undef
assets/js/spellchecker.js
Outdated
for(var sugg = 0; sugg < data[tokenIndex].sugg.length; sugg++) { | ||
content[splitWords[originalWordsIndex]] += '<a href="#" class="spellCheckerListItem">' + | ||
data[tokenIndex].sugg[sugg][0] + '</a>'; | ||
content[splitWords[originalWordsIndex]] += '</div>'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'content' is not defined no-undef
'splitWords' is not defined no-undef
assets/js/spellchecker.js
Outdated
splitWords[originalWordsIndex] + '>' + splitWords[originalWordsIndex] + '</span>'); | ||
content[splitWords[originalWordsIndex]] = '<div class="spellCheckerList">'; | ||
for(var sugg = 0; sugg < data[tokenIndex].sugg.length; sugg++) { | ||
content[splitWords[originalWordsIndex]] += '<a href="#" class="spellCheckerListItem">' + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'content' is not defined no-undef
'splitWords' is not defined no-undef
assets/js/spellchecker.js
Outdated
} | ||
$('#spellCheckerInput').html($('#spellCheckerInput').html() + ' <span class="spellError" id=' + | ||
splitWords[originalWordsIndex] + '>' + splitWords[originalWordsIndex] + '</span>'); | ||
content[splitWords[originalWordsIndex]] = '<div class="spellCheckerList">'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'content' is not defined no-undef
'splitWords' is not defined no-undef
assets/js/spellchecker.js
Outdated
continue; | ||
} | ||
$('#spellCheckerInput').html($('#spellCheckerInput').html() + ' <span class="spellError" id=' + | ||
splitWords[originalWordsIndex] + '>' + splitWords[originalWordsIndex] + '</span>'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'splitWords' is not defined no-undef
assets/js/spellchecker.js
Outdated
var originalWordsIndex = 0; | ||
for(var tokenIndex = 0; tokenIndex < data.length; tokenIndex++) { | ||
if(data[tokenIndex].known === true) { | ||
$('#spellCheckerInput').html($('#spellCheckerInput').html() + ' ' + splitWords[originalWordsIndex]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'splitWords' is not defined no-undef
c52e3ae
to
02ab612
Compare
b5960d5
to
1f1165c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Initial run through, looking on the right track. Haven't tested it yet though.
I don't have too much time right now so I couldn't take a more detailed look.
Makefile
Outdated
@@ -46,7 +46,8 @@ JSFILES= \ | |||
assets/js/translator.js \ | |||
assets/js/analyzer.js \ | |||
assets/js/generator.js \ | |||
assets/js/sandbox.js | |||
assets/js/sandbox.js \ | |||
assets/js/spellchecker.js |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Spacing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sushain97 this is weird, it looks fine in the actual file, but looks wrong here.
assets/js/localization.js
Outdated
@@ -13,7 +13,8 @@ var localizedLanguageCodes /*: {[string]: string} */ = {}, localizedLanguageName | |||
|
|||
/* global config, getPairs, getGenerators, getAnalyzers, persistChoices, getURLParam, cache, ajaxSend, ajaxComplete, sendEvent, | |||
srcLangs, dstLangs, generators, analyzers, readCache, modeEnabled, populateTranslationList, populateGeneratorList, | |||
populateAnalyzerList, analyzerData, generatorData, curSrcLang, curDstLang, restoreChoices, refreshLangList, onlyUnique */ | |||
populateAnalyzerList, analyzerData, generatorData, curSrcLang, curDstLang, restoreChoices, refreshLangList, onlyUnique |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing a comma?
assets/js/localization.js
Outdated
'translation': getPairs, | ||
'generation': getGenerators, | ||
'analyzation': getAnalyzers, | ||
'spellchecker': getSpellers |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it just me or is something off, shouldn't it be 'spell checking'? Otherwise, 'translation' would be 'translator', etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(and in other analagous places)
index.html.in
Outdated
</div> | ||
<div class="col-sm-4"> | ||
<select class="form-control spellCheckerMode" id="secondarySpellCheckerMode" name="secondarySpellCheckerMode"> | ||
<option> </option> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Spacing
assets/js/spellchecker.js
Outdated
} | ||
|
||
function spellCheckerNotAvailable(data) { | ||
$('#spellCheckerInput').append($('<div></div>').text(' ')); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the point of .text(' ')
?
Also, please add the flow annotation to |
Tabs vs spaces
…On Jan 6, 2018 1:36 PM, "Jonathan Pan" ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In Makefile
<#253 (comment)>
:
> @@ -46,7 +46,8 @@ JSFILES= \
assets/js/translator.js \
assets/js/analyzer.js \
assets/js/generator.js \
- assets/js/sandbox.js
+ assets/js/sandbox.js \
+ assets/js/spellchecker.js
@sushain97 <https://github.com/sushain97> this is weird, it looks fine in
the actual file, but looks wrong here.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#253 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AEBEfglOyBpzEr5YHiWD_qo8YLQKYwmaks5tH8s-gaJpZM4RUHst>
.
|
@sushain97 I don't think it should be called |
939d8b6
to
f3537ab
Compare
Also, please undo your test data commit since I got this up with real data. |
abbbab9
to
abd87e2
Compare
assets/js/spellchecker.js
Outdated
var currentSpellCheckerRequest; | ||
|
||
/* exported getSpellers, populateSecondarySpellCheckerList */ | ||
/* global config, modeEnabled, persistChoices, readCache, ajaxSend, ajaxComplete, filterLangPairList, allowedLang, analyzers, cache, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'analyzers' is defined but never used no-unused-vars
2f53ae9
to
cec0819
Compare
7d9a9e5
to
b541da3
Compare
assets/js/spellchecker.js
Outdated
deferred.resolve(); | ||
} | ||
});*/ | ||
var data = {"eng":"test1"}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Strings must use singlequote quotes
Missing space before value for key 'eng' key-spacing
assets/js/localization.js
Outdated
if(modeEnabled('spellchecking')) { | ||
populatePrimarySpellCheckerList(spellerData); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Trailing spaces not allowed no-trailing-spaces
assets/js/localization.js
Outdated
@@ -304,6 +310,10 @@ function localizeLanguageNames(localizedNamesFromJSON) { | |||
if(modeEnabled('analyzation')) { | |||
populateAnalyzerList(analyzerData); | |||
} | |||
if(modeEnabled('spellchecking')) { | |||
populatePrimarySpellCheckerList(spellerData); | |||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Trailing spaces not allowed no-trailing-spaces
assets/js/localization.js
Outdated
@@ -13,7 +13,8 @@ var localizedLanguageCodes /*: {[string]: string} */ = {}, localizedLanguageName | |||
|
|||
/* global config, getPairs, getGenerators, getAnalyzers, persistChoices, getURLParam, cache, ajaxSend, ajaxComplete, sendEvent, | |||
srcLangs, dstLangs, generators, analyzers, readCache, modeEnabled, populateTranslationList, populateGeneratorList, | |||
populateAnalyzerList, analyzerData, generatorData, curSrcLang, curDstLang, restoreChoices, refreshLangList, onlyUnique */ | |||
populateAnalyzerList, populatePrimarySpellCheckerList, analyzerData, generatorData, spellerData, curSrcLang, curDstLang, restoreChoices, refreshLangList, onlyUnique, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Line 16 exceeds the maximum line length of 140 max-len
9b53135
to
1fc6ce5
Compare
assets/js/spellchecker.js
Outdated
} | ||
$('.spellError').each(function () { | ||
var currentTokenId = this.id; | ||
if(content[currentTokenId].indexOf("spellCheckerListItem") !== -1) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Strings must use singlequote quotes
d501dc1
to
649fb8c
Compare
@@ -13,7 +13,8 @@ var URL_PARAM_Q_LIMIT = 1300, | |||
'#translation': 'q', | |||
'#webpageTranslation': 'qP', | |||
'#analyzation': 'qA', | |||
'#generation': 'qG' | |||
'#generation': 'qG', | |||
'#spellchecking': 'qS' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sushain97 This currently conflicts with sandbox. What should I change this to?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't really think sandbox needs one...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sushain97 It at least needs something, otherwise errors start popping up and breaks the UI.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or let's just make it qSand
.
649fb8c
to
2c0791e
Compare
hello everyone i am A frontend Devolper And i Want to be part of Your Community In This GSOC 2023 Name :- Abhijit Dengale |
spellchecker.js