SpellChecker Interface #176
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
Author
|
Travis is going to be very very unhappy. Please don't look at the build results :P |
Contributor
Author
|
@sushain97 , Um, the Travis shows 2 errors. I am using |
Member
|
I will be AFK till late this evening. You should be able to Google the
complexity error. It's an ESLint rule.
…On Jul 27, 2017 12:26 PM, "Monish Godhia" ***@***.***> wrote:
@sushain97 <https://github.com/sushain97> , Um, the Travis shows 2
errors. I am using currentSpellCheckerRequest in callApy but I do not
understand the error. There is one more error about complexity 37. Could
you help me in understanding what these are and how they can be resolved?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#176 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AEBEfmFyNA33B1jEAxiG4HERDwrPKXVSks5sSORigaJpZM4OkUNl>
.
|
sushain97
reviewed
Aug 19, 2017
Member
sushain97
left a comment
sushain97
left a comment
There was a problem hiding this comment.
Lots of code! Some comments from first passthrough. I'm a bit concerned that there's a lot of repeated code here...
| resize: both; | ||
| } | ||
|
|
||
| .spellcheckVisible .spellError { |
Member
There was a problem hiding this comment.
.spellcheckVisible => .spellcheckerVisible
| border-bottom: 1px solid #f00; | ||
| } | ||
|
|
||
| .popover-content { /* sass-lint:disable-line class-name-format */ |
Member
There was a problem hiding this comment.
Can we make this selector more specific?
| margin-left: 30px; | ||
| } | ||
|
|
||
| /* SpellCheker */ |
| width: 110%; | ||
| } | ||
|
|
||
| .list-group { /* sass-lint:disable-line class-name-format */ |
Member
There was a problem hiding this comment.
Same with this and the next two. Basically my ask is to wrap it in a more specific class.
| clearTimeout(timer); | ||
| check(); | ||
| }); | ||
|
|
| success: function (data) { | ||
| var originalWordsIndex = 0; | ||
| for(var tokenIndex = 0; tokenIndex < data.length; tokenIndex++) { | ||
| if(data[tokenIndex]['known'] === true) { // eslint-disable-line dot-notation |
Member
There was a problem hiding this comment.
Why did you disable eslint here?
| } | ||
| $('.spellError').each(function () { | ||
| var currentTokenId = this.id; | ||
| $(this).popover({animation: false, placement: 'bottom', trigger: 'manual', html: true, |
sushain97
requested changes
Aug 19, 2017
Open
JPJPJPOPOP
added a commit
to JPJPJPOPOP/apertium-html-tools
that referenced
this pull request
Jan 5, 2018
JPJPJPOPOP
added a commit
to JPJPJPOPOP/apertium-html-tools
that referenced
this pull request
Jan 5, 2018
JPJPJPOPOP
added a commit
to JPJPJPOPOP/apertium-html-tools
that referenced
this pull request
Jan 5, 2018
JPJPJPOPOP
added a commit
to JPJPJPOPOP/apertium-html-tools
that referenced
this pull request
Jan 5, 2018
JPJPJPOPOP
reviewed
Jan 6, 2018
| $(document).ready(function () { | ||
| restoreChoices('spellchecker'); | ||
| var timer, timeout = 2000; | ||
| $('#spellChekerForm').submit(function () { |
Contributor
There was a problem hiding this comment.
$('#spellChekerForm') => $('#spellCheckerForm')
JPJPJPOPOP
added a commit
to JPJPJPOPOP/apertium-html-tools
that referenced
this pull request
Jan 7, 2018
JPJPJPOPOP
added a commit
to JPJPJPOPOP/apertium-html-tools
that referenced
this pull request
Jan 9, 2018
JPJPJPOPOP
added a commit
to JPJPJPOPOP/apertium-html-tools
that referenced
this pull request
Jan 21, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The current logic of mapping the suggestions on the frontend is based up on indices. I have a better idea in mind where I would make use of
tokenreturned from backend to do a string comparison on the frontend. However, this PR till then, could be reviewed and improved upon.