Skip to content
This repository was archived by the owner on Sep 6, 2021. It is now read-only.

#11801 - Restricted Font Size Input To Only Valid Entries #13123

Merged
merged 14 commits into from
Mar 3, 2017
Merged

#11801 - Restricted Font Size Input To Only Valid Entries #13123

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
df77e15
Restricted font size input to valid entries.
nyteksf Feb 24, 2017
7689017
#11801 - Restricted font size input to valid entries.
nyteksf Feb 24, 2017
2143187
#11801 - Added @zaggino's prescribed changes
nyteksf Feb 25, 2017
f7a0cab
Deleted trailing spaces, and added missed change
nyteksf Feb 25, 2017
01ae3a8
#11801 - Removed line of whitespace
nyteksf Feb 25, 2017
8131ef4
#11801 - Added inner quotes
nyteksf Feb 25, 2017
337946f
#11801 - Implemented @ficristo's Requested Changes: HTML5 pattern att…
nyteksf Feb 27, 2017
9a6f12c
#11801 - Edited tag value for clarity
nyteksf Feb 27, 2017
a5253e7
#11801 - Added @ficristo's Changes: Exposed RegExp To Be Used On HTML
nyteksf Mar 3, 2017
8839926
#11801 - Added @zaggino's Stylistic Changes
nyteksf Mar 3, 2017
fdfdcc5
#11801 - Trimmed Edge Cases From Font Size Validating RegExp
nyteksf Mar 3, 2017
febb9cb
#11801 - Corrected Format Of RegExp
nyteksf Mar 3, 2017
e337dc7
slash should be escaped
zaggino Mar 3, 2017
737f086
this variable was never used
zaggino Mar 3, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/config.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
"extension_url": "https://s3.amazonaws.com/extend.brackets/{0}/{0}-{1}.zip",
"linting.enabled_by_default": true,
"build_timestamp": "",
"healthDataServerURL": "https://healthdev.brackets.io/healthDataLog"
"healthDataServerURL": "https://health.brackets.io/healthDataLog"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you need to revert this change (it applies automatically after doing npm install, but we don't want it in PRs)

},
"name": "Brackets",
"version": "1.9.0-0",
Expand Down
6 changes: 3 additions & 3 deletions src/htmlContent/themes-settings.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,20 +30,20 @@ <h1 class="dialog-title">{{Strings.THEMES_SETTINGS}}</h1>
<div class="control-group">
<label class="control-label">{{Strings.FONT_SIZE}}:</label>
<div class="controls">
<input type="text" data-target="fontSize" value="{{settings.fontSize}}">
<input type="text" id="font-size-input" data-target="fontSize" value="{{settings.fontSize}}">
</div>
</div>

<div class="control-group">
<label class="control-label">{{Strings.FONT_FAMILY}}:</label>
<div class="controls">
<input type="text" data-target="fontFamily" value="{{settings.fontFamily}}">
<input type="text" id="font-family-input" data-target="fontFamily" value="{{settings.fontFamily}}">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't need to introduce id's here, you can use .on("input", "[data-target=fontFamily]", function () {
https://www.w3schools.com/jquery/sel_attribute_equal_value.asp

</div>
</div>
</form>
</div>
<div class="modal-footer">
<button class="dialog-button btn" data-button-id="cancel">{{Strings.CANCEL}}</button>
<button class="dialog-button btn primary" data-button-id="save">{{Strings.DONE}}</button>
<button class="dialog-button btn primary" id="done" data-button-id="save">{{Strings.DONE}}</button>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

id done is not very good, you should be more specific when introducing id's, something like theme-settings-done-btn would be more appropriate

</div>
</div>
21 changes: 19 additions & 2 deletions src/view/ThemeSettings.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,24 @@ define(function (require, exports, module) {
var attr = $target.attr("data-target");
newSettings[attr] = $target.is(":checked");
})
.on("input", "[data-target]:text", function () {
.on("input", "#font-size-input", function () {
var $target = $(this);
var $input = $target.val();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$ prefix for variables indicates that it's a jquery (or a DOM) element
in this case $input is a string and should really be named input or even better targetValue

var validInput = /^[\d\.]+(p|px|e|em){0,1}$/g;
var btn = document.getElementById("done");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add document to the globals please: https://github.com/adobe/brackets/blob/master/.eslintrc.json#L52


// Make sure that the font size is expressed in terms we can handle (px or em). If not, 'done' button disabled until input corrected.

if ($input.search(validInput) === -1) {
($input.length === 0) ? btn.disabled=false : btn.disabled=true;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

btn.disabled = $input.length !== 0 is better

return false;
}else{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

}else{ after return doesn't make sense, just close the if with } and continue normally

if(btn.disabled) btn.disabled = false;
var attr = $target.attr("data-target");
newSettings[attr] = $input;
}
})
.on("input", "#font-family-input", function () {
var $target = $(this);
var attr = $target.attr("data-target");
newSettings[attr] = $target.val();
Expand All @@ -126,7 +143,7 @@ define(function (require, exports, module) {
// Figure out if the setting is in the ViewCommandHandlers, which means it is
// a font setting
setterFn = "set" + setting[0].toLocaleUpperCase() + setting.substr(1);
if (typeof ViewCommandHandlers[setterFn] === 'function') {
if (typeof ViewCommandHandlers[setterFn] === "function") {
ViewCommandHandlers[setterFn](newSettings[setting]);
}
}
Expand Down