[CLOSED] #11801 - Restricted Font Size Input To Only Valid Entries

[core-ai-bot]

Issue by nyteksf
Friday Feb 24, 2017 at 22:28 GMT
Originally opened as adobe/brackets#13123

---

Related to #11801.

Text was able to be inputted into Font Size within Themes Settings (e.g. "View > Themes...") even when it included non-ASCII characters or was in a generally unusable format.

Corrected this issue by ensuring all font sizes are to begin with a number, and are completed by either "px" or "em" only. 'Done' button remains disabled until said format is adhered to correctly--at which time the functionality of said button becomes restored allowing input to be saved.

@zaggino

---

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

[core-ai-bot]

Comment by zaggino
Friday Feb 24, 2017 at 23:29 GMT

---

also make sure command grunt eslint in your branch doesn't throw any errors

[core-ai-bot]

Comment by ficristo
Saturday Feb 25, 2017 at 13:21 GMT

---

Out of curiosity, any reason to not use the pattern attribute?

[core-ai-bot]

Comment by zaggino
Sunday Feb 26, 2017 at 04:33 GMT

---

Yeah, using pattern ( https://www.w3schools.com/tags/att_input_pattern.asp ) directly on the input would probably be better.

[core-ai-bot]

Comment by mohanad1980
Sunday Feb 26, 2017 at 05:32 GMT

---

hello can you contact me mohanad.rahimi1@gmail.com

[core-ai-bot]

Comment by nyteksf
Monday Feb 27, 2017 at 08:28 GMT

---

Thanks, guys, for your speedy replies. And the link. To reply now to@ficristo: that HTML5 pattern attribute was actually wholly new to me. I've learned and relearned a bunch of neat stuff here in just a few reviewer comments. And I have to agree with you both that "pattern" is much better in usage. Thanks again! Please feel free to let me know if anything else would be problematic.

[core-ai-bot]

Comment by zaggino
Monday Feb 27, 2017 at 08:38 GMT

---

fine with me,@ficristo ?

[core-ai-bot]

Comment by ficristo
Monday Feb 27, 2017 at 08:54 GMT

---

It seems fine for me.
Ideally we could expose this regex: https://github.com/adobe/brackets/blob/d6ae906810b8e6afb43c88edcf0397b1dc1b0119/src/view/ViewCommandHandlers.js#L300
and use it on the html.

[core-ai-bot]

Comment by zaggino
Monday Feb 27, 2017 at 09:05 GMT

---

@nyteksf to do what@ficristo wrote, you'd have to edit this file https://github.com/nyteksf/brackets/blob/9a6f12cf259276ebd163e70266030377df67eb2b/src/view/ViewCommandHandlers.js and make this https://github.com/adobe/brackets/blob/d6ae906810b8e6afb43c88edcf0397b1dc1b0119/src/view/ViewCommandHandlers.js#L300 an exported variable.

Then here https://github.com/nyteksf/brackets/blob/9a6f12cf259276ebd163e70266030377df67eb2b/src/view/ThemeSettings.js#L82 you'd pass the ViewCommandHandlers.validFontRegex (or however you call it) to Mustache and render it here https://github.com/nyteksf/brackets/blob/9a6f12cf259276ebd163e70266030377df67eb2b/src/htmlContent/themes-settings.html#L33 like {{ fontPattern }} or something

Hope that's clear enough,@ficristo correct me if I'm wrong

[core-ai-bot]

Comment by petetnt
Monday Feb 27, 2017 at 12:25 GMT

---

LGTM, but we should accept at least rem values and percentages too, no?

[core-ai-bot]

Comment by zaggino
Monday Feb 27, 2017 at 12:32 GMT

---

Current regexp which is used deeper ( https://github.com/adobe/brackets/blob/d6ae906810b8e6afb43c88edcf0397b1dc1b0119/src/view/ViewCommandHandlers.js#L300 ) seems to only allow px|em ... do we want to change this?

[core-ai-bot]

Comment by nyteksf
Friday Mar 03, 2017 at 01:35 GMT

---

Thank you, guys. I have had to fiddle around with the RegExp a little to make the prescribed changes work. It seems backslashes need to be escaped to be exported successfully with mustache--having tried the various remedies in the docs that seemed to apply--and this causes the RegExp constructor used for the non-exported segment to be passed something unintended in that case (e.g., \\ and d instead of \d). Likewise, to use RegExp literally, it seems would require .slice()ing off the foreslashes to be made compatible with the HTML5 Pattern Attribute for export thereunto.

That said, rather than attempting to escape any characters, I have switched the RegExp in question to "[0-9.]..." instead of "[\d.]". But everything else should be as asked for to my awareness. Please let me know what you think in any case. Thanks very much. :)

@zaggino@ficristo

[core-ai-bot]

Comment by nyteksf
Friday Mar 03, 2017 at 02:22 GMT

---

Whoops! Well, okay: there you go. Thanks again for a speedy reply!

@zaggino

[core-ai-bot]

Comment by zaggino
Friday Mar 03, 2017 at 02:31 GMT

---

Testing this, it allows input like:
12.00.00px
can we please change regexp to be maybe:
"^[0-9]+(\\.[0-9]+)?(px|em)$"

[core-ai-bot]

Comment by zaggino
Friday Mar 03, 2017 at 02:33 GMT

---

Done with testing, just change this one and I'm merging this (promise 😄 )

[core-ai-bot]

Comment by nyteksf
Friday Mar 03, 2017 at 05:16 GMT

---

Okay. I'm going out on a limb here: so of course please do correct me if my suggestion happens to be wrong. I added your change to the original deeper RegExp. Then in doing my own testing, I had found that by comparison to the original it worked almost perfectly. That RegExp in question, however, didn't handle cases like ".5em", without the user first adding a zero to make it like "0.5em". That isn't a problem, I'm sure. My change seems like a small change indeed with that being said--but I figured I could change it still for sake of ease of use. And now it should handle cases like "0.5em" as well as ".5em". Please let me know if it works as it seems to on my end. And thanks again for all your help. :)))

@zaggino