Adding JSLint passed inline color editor #2010
Conversation
ghost
assigned
|
We should add TinyColor to the NOTICE file in the root of the repo either way, though. |
|
Related note: all the .js and .css files need to have our standard MIT license header added. |
|
Super stoked for this. Thanks Raymond. |
There was a problem hiding this comment.
Should align the "=" signs, like in our other modules
|
I'm seeing a bug where hitting Esc to close the color picker doesn't restore keyboard focus to the host editor. I'm not entirely sure how that works for regular inline editors, so I'd suggest filing a bug to tackle after this is merged (but before the user story is DoD'ed). @RaymondLim, can you do that? |
There was a problem hiding this comment.
Remove the return keyword (this seems to be a functional-programming CoffeeScript-ism)
…erwise, we won't be able to remove the current color picker with the already deleted bookmark.
… not use a variable to hold the colorPicker that we want to delete. Instead, we can just delete it first and then clear bookmarks.
|
Just reviewed all the updates -- I'll post all my comments in one batch now. |
src/document/DocumentManager.js
Outdated
There was a problem hiding this comment.
We might not want to make this an official public API yet -- not sure we have time to think through exactly what the right API is that will serve all the various use cases (e.g. warning markers, etc.). For now it seems ok to have the color editor code just reference _codeMirror directly.
There was a problem hiding this comment.
OK, I removed it and just directly call _codeMirror in color editor code.
|
One other comment: can we rename the "helper" folder to "thirdparty"? |
|
Done re-reviewing |
|
For consistency I go ahead and update "helper" to "thirdparty". |
-Removed _colorPickers map and the corresponding code. -Added color swatches to tab loop. -Renamed some handlers xxxFocus() -> xxxKeyDown(). -Removed some redundent "return" keyword.
|
Alright, I think we're in a good enough state to merge! |
Adding inline color editor to core
No description provided.