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] Fixes to hover preview color RegExp #3325

Open
core-ai-bot opened this issue Aug 29, 2021 · 9 comments
Open

[CLOSED] Fixes to hover preview color RegExp #3325

core-ai-bot opened this issue Aug 29, 2021 · 9 comments

Comments

@core-ai-bot
Copy link
Member

Issue by redmunds
Monday Apr 22, 2013 at 23:09 GMT
Originally opened as adobe/brackets#3540


The RegExp is difficult to diff, so here are my changes. I made 3 changes:

  1. For [CLOSED] [js code hints] Should not have horizontal scroll bar in parameter hint list. #3454, add "1.0" list of possible to alpha values for rgba()/hsla().
  2. For [CLOSED] [quick view] Check color boundaries for hyphens #3455 The hsl()/hsla() functions now accept integer up to 999 (degrees) for the first parameter. According to the spec (http://www.w3.org/TR/css3-color/#hsl-color), "As an angle, it implicitly wraps around...". It would be easy to add more digits, if desired.
  3. I noticed that zero or one space chars was accepted for optional whitespace (" ?"), so I changed it to zero or more whitespace chars ("\s*") to be more forgiving (and accurate).

redmunds included the following code: https://github.com/adobe/brackets/pull/3540/commits

@core-ai-bot
Copy link
Member Author

Comment by gruehle
Tuesday Apr 23, 2013 at 19:47 GMT


Initial review complete. It would be nice to split the regex into multiple lines and move into a core file so it can be shared with InlineColorEditor.

@core-ai-bot
Copy link
Member Author

Comment by redmunds
Tuesday Apr 23, 2013 at 22:44 GMT


@gruehle The color regex used by the InlineColorEditor does not have teh named colors. Is it OK if it does, or will that cause problems?

@core-ai-bot
Copy link
Member Author

Comment by RaymondLim
Tuesday Apr 23, 2013 at 23:07 GMT


I do have color regex in my branch rlim/improved-color-picker which construct a named color array from tinycolor library.

    InlineColorEditor.colorNameArray = $.map(tinycolor.names, function (value, key) {
        return "\\b" + key + "\\b";
    });

@core-ai-bot
Copy link
Member Author

Comment by gruehle
Tuesday Apr 23, 2013 at 23:07 GMT


The

@core-ai-bot
Copy link
Member Author

Comment by gruehle
Tuesday Apr 23, 2013 at 23:08 GMT


Whoops, accidentally closed. Sorry about that!

@core-ai-bot
Copy link
Member Author

Comment by gruehle
Wednesday Apr 24, 2013 at 00:13 GMT


@redmunds - it looks like some changes to StringUtils got added with your last commit. These are causing the Travis failure.

@core-ai-bot
Copy link
Member Author

Comment by TomMalbran
Wednesday Apr 24, 2013 at 00:18 GMT


The color regular expression was moved to the StringUtils as an array, but without the double escapes, which caused the JSHint error.

@core-ai-bot
Copy link
Member Author

Comment by redmunds
Wednesday Apr 24, 2013 at 02:13 GMT


I tried to break the color regex into an array of regexes, but I couldn't quite get it working, so I punted for now.

I implemented a small performance improvement: don't look for a color if we already have a gradient.

I fixed a couple more problems with color regex:

  • make functions start on word boundary
  • remove unnecessary word boundary check from numbers

Copied changes to InlineColorEditor color regex (everything except named colors).

Ready for final review.

@core-ai-bot
Copy link
Member Author

Comment by gruehle
Wednesday Apr 24, 2013 at 02:39 GMT


Thanks for trying the regexp split. Too bad it didn't work out.

Everything else looks good.

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