[CLOSED] Adding JSLint passed inline color editor

[core-ai-bot]

Issue by RaymondLim
Wednesday Oct 31, 2012 at 22:07 GMT
Originally opened as adobe/brackets#2010

---

---

RaymondLim included the following code: https://github.com/adobe/brackets/pull/2010/commits

[core-ai-bot]

Comment by peterflynn
Thursday Nov 01, 2012 at 20:33 GMT

---

"tinycolor-min.js" is a third-party library. I wonder if we should move this into the core src/thirdparty folder, or leave it under the extensions/default subfolder? @njx,@gruehle,@pthiess, any thoughts on that?

[core-ai-bot]

Comment by peterflynn
Thursday Nov 01, 2012 at 20:36 GMT

---

We should add TinyColor to the NOTICE file in the root of the repo either way, though.

[core-ai-bot]

Comment by peterflynn
Thursday Nov 01, 2012 at 20:37 GMT

---

Related note: all the .js and .css files need to have our standard MIT license header added.

[core-ai-bot]

Comment by GarthDB
Thursday Nov 01, 2012 at 20:40 GMT

---

Super stoked for this.

Thanks Raymond.

[core-ai-bot]

Comment by peterflynn
Thursday Nov 01, 2012 at 21:04 GMT

---

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?

[core-ai-bot]

Comment by peterflynn
Thursday Nov 01, 2012 at 23:18 GMT

---

Done reviewing.

Some of the issues flagged above are side effects of the CoffeeScript->JS conversion, but others I think were probably problematic in the original code too.

[core-ai-bot]

Comment by GarthDB
Thursday Nov 01, 2012 at 23:22 GMT

---

I would blame more on the author then on the coffeescript.

Garth Braithwaite
Sent with Sparrow (http://www.sparrowmailapp.com)

On Thursday, November 1, 2012 at 4:19 PM, Peter Flynn wrote:

Done reviewing.
Some of the issues flagged above are side effects of the CoffeeScript->JS conversion, but others I think were probably problematic in the original code too.

—
Reply to this email directly or view it on GitHub (adobe/brackets#2010 (comment)).

[core-ai-bot]

Comment by peterflynn
Thursday Nov 01, 2012 at 23:55 GMT

---

@GarthDB: well you can at least blame my 5 million comments re unneeded return statements on CoffeeScript :-)

[core-ai-bot]

Comment by GarthDB
Friday Nov 02, 2012 at 00:17 GMT

---

That is automatic. I wonder if there is a syntax that overrides that
behavior.

On Thursday, November 1, 2012, Peter Flynn wrote:

@GarthDB https://github.com/GarthDB: well you can at least blame my 5
million comments re unneeded return statements on CoffeeScript :-)

—
Reply to this email directly or view it on GitHubhttps://github.com/adobe/brackets/pull/2010#issuecomment-10000559.

[core-ai-bot]

Comment by gruehle
Friday Nov 02, 2012 at 20:26 GMT

---

Regarding tinycolor.js, I think we should leave it in the extension for now.

I would prefer to keep 3rd party libraries in extensions until we have a need to move them into Brackets core. I don't want to end up with a bunch of libraries in core that have little or no use in core code.

[core-ai-bot]

Comment by RaymondLim
Saturday Nov 03, 2012 at 04:57 GMT

---

Changes are pushed. I put some line comments for those that I can't change.

[core-ai-bot]

Comment by peterflynn
Tuesday Nov 13, 2012 at 23:25 GMT

---

Just reviewed all the updates -- I'll post all my comments in one batch now.

[core-ai-bot]

Comment by peterflynn
Wednesday Nov 14, 2012 at 00:14 GMT

---

One other comment: can we rename the "helper" folder to "thirdparty"?

[core-ai-bot]

Comment by peterflynn
Wednesday Nov 14, 2012 at 00:15 GMT

---

Done re-reviewing

[core-ai-bot]

Comment by RaymondLim
Wednesday Nov 14, 2012 at 20:00 GMT

---

For consistency I go ahead and update "helper" to "thirdparty".