Skip to content
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

[CLOSED] #7723 - Adding Support For '0x' Notation Color Format #11157

Open
core-ai-bot opened this issue Aug 30, 2021 · 10 comments
Open

[CLOSED] #7723 - Adding Support For '0x' Notation Color Format #11157

core-ai-bot opened this issue Aug 30, 2021 · 10 comments

Comments

@core-ai-bot
Copy link
Member

Issue by nyteksf
Tuesday Mar 07, 2017 at 01:44 GMT
Originally opened as adobe/brackets#13154


Hello. Could somebody please point me in the right direction from here where I am stuck? I have implemented the RegExp used to find "0x..." format colors which are entered as input. I have also added the associated button, and plugged in the related code that it might be used (for the most part). However, I'm now having trouble linking everything up in a non-hackish way (or at all) in large part because tinycolor doesn't recognize 0x notation. As a result I cannot get the color swatches to work, or 0x notation to be easily recognized by the given methods of tinycolor like .getFormat() which is for example used with a switch case in Brackets for control flow. Thanks in advance to anyone who can lend me their support! :)

@zaggino


nyteksf included the following code: https://github.com/adobe/brackets/pull/13154/commits

@core-ai-bot
Copy link
Member Author

Comment by petetnt
Wednesday Mar 08, 2017 at 21:02 GMT


Hi@nyteksf,

if the issue is that tinycolor doesn't support 0x format, I would suggest that you always convert any potential 0x values to RGB first. Basically kinda like you have at your checkIf0xNotation, but much more simpler. Something like:

function asRGB(color) {
  if (/0x/.test(color)) {
    return color.replace("0x", "#");
  }

  return color;
}

/** somewhere down the line where tinycolor is called **/
tinycolor(asRGB(this._getColor())

// when and only when you need the actual color value (for example presentation) convert it to 0x format: 

if (this._format === "0x") {
  return color.toHexString().replace("#", "0x");
}

return color.toHexString();

@core-ai-bot
Copy link
Member Author

Comment by ficristo
Thursday Mar 09, 2017 at 09:26 GMT


@nyteksf if TinyColor doesn't support the 0x notation, I think you should file an issue to https://github.com/bgrins/TinyColor to ask support for it.
(From what I saw newer version of the library still don't support the 0x notation, but you could double check)
ATM that repo doesn't seem very active so maybe nothing will come out anytime soon, but if they aren't aware of the issue it will never be fixed.

@core-ai-bot
Copy link
Member Author

Comment by zaggino
Sunday Mar 12, 2017 at 23:26 GMT


@nyteksf added a few comments, please have a look

@core-ai-bot
Copy link
Member Author

Comment by nyteksf
Monday Mar 13, 2017 at 05:21 GMT


Okay. Firstly, I just want to take a moment to thank you guys, once again, for helping me--each one of these feedback sessions has been worth at least a million articles or book pages by dint of contrast.

That said, in response to my most recent changes, I think I have satisfied your associated change requests. Or no? Please let me know if otherwise, and again, I will hop to that so I can get onto the next one. Good times. :)

@core-ai-bot
Copy link
Member Author

Comment by zaggino
Monday Mar 13, 2017 at 06:00 GMT


Just two of my comments, lets wait for@petetnt to re-check this too.

@core-ai-bot
Copy link
Member Author

Comment by nyteksf
Monday Mar 13, 2017 at 11:56 GMT


Okay. The latest change requests have been fulfilled, or so I believe. As always--please don't hesitate to let me know if there're any more in mind. Otherwise, thanks again. And see you again soon. :)

@zaggino@petetnt

@core-ai-bot
Copy link
Member Author

Comment by zaggino
Monday Mar 13, 2017 at 23:29 GMT


Looks good now except one place where the code is incorrectly indented, it'd be a good thing to install brackets-eslint extension and fix the warnings in the code you've modified (you don't need to fix warnings in code you didn't touch of course, we know there're some, but we're trying to get the codebase to be more consistent)

@core-ai-bot
Copy link
Member Author

Comment by nyteksf
Tuesday Mar 14, 2017 at 11:26 GMT


:)
Thanks!
@petetnt

@core-ai-bot
Copy link
Member Author

Comment by zaggino
Tuesday Mar 14, 2017 at 11:53 GMT


Merged, thanks@nyteksf

@core-ai-bot
Copy link
Member Author

Comment by petetnt
Tuesday Mar 14, 2017 at 11:54 GMT


Great job@nyteksf! Thanks for this contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant