Logic to make sure scrollbar processing only happens in the editor space... #8493

MiguelCastillo · 2014-07-22T04:34:06Z

.... Also fixed issue with scrollbar regex.

Recognize existing indentation when indenting close braces

…missed-a-spot

…ace. Also fixed issue with scrollbar regex. #8482

MiguelCastillo · 2014-07-22T04:37:22Z

Guys, so the issue is actually a regression I introduced when I removed parts of the scrollbars regex in order to make jslint happy. :/ The warnings in the regex are back, but the regex is working as expected again. Not sure what to do about the warnings, so any help would be nice :)

MiguelCastillo · 2014-07-22T04:43:27Z

BTW, I added the dark class to DOM in the element where platform exists...

TomMalbran · 2014-07-22T05:09:29Z

To make JSLint happy you can either try to escape ^ (which does not always work) or just add regexp: true to the list of JSLint directives so that it stops complaining about it.

peterflynn · 2014-07-22T05:13:49Z

We use regexp: true in a few other files, so it's definitely fine to do that. The motivation behind that warning seems to be specific to cases where you're using regexps to validate form fields, so it isn't useful to us here.

Make negative areas of selection triangle transparent (& other fixes)

MiguelCastillo · 2014-07-22T13:12:42Z

@peterflynn @TomMalbran I knew that! :D Thank you guys

Fonts api with Themes integrated

dangoor · 2014-07-22T15:55:20Z

src/view/ThemeManager.js

            .then(function (cssContent) {
+                $("[class|=platform]").toggleClass("dark", theme.dark);


why not just set this on body?

Yeah will change that. I was trying to keep this in the spirit of the platform- issue we were discussing yesterday. But body is definitely my preference to.

There's a subtle issue here. If options.theme is undefined, then theme.dark will be undefined rather than false. Changing the line in the Theme constructor like so will fix it:

this.dark = options.theme !== undefined && options.theme.dark === true;

Yeah I will change that to be air tight, but the toggleClass will behave the same either way.

If you change from one light theme to another light theme, it would toggle the dark class to dark on the second one...

Easier:

this.dark = (options.theme && options.theme.dark) || false;

Yeah that will work too... options.theme at that point couldn't be undefined because it would have already crashes when in loadPackage.

And at the point !!options.theme.dark would be sufficient

…ace. Also fixed issue with scrollbar regex. #8482

…s-8482

dangoor · 2014-07-22T16:21:54Z

Hmm, @peterflynn's comment from yesterday evening suggesting that we move the dark theme scrollbars into core because they're finicky makes me wonder about something. If we move them into core, when you turn off custom scrollbars and you're using a dark theme, it will actually revert not to system scrollbars but the default dark scrollbars that Brackets provides. That's probably okay, but would be confusing if the Theme Settings dialog says "Use system scrollbars" rather than the current "Use custom scrollbars" as in #8384

dangoor · 2014-07-22T16:22:49Z

That comment I just put probably belongs in #8384 rather than here. I think the change here will be fine for allowing custom scrollbars in editor themes.

…is found. Also added coercion to make theme.dark always be a boolean

into themes-8482 Conflicts: src/view/ThemeManager.js

dangoor · 2014-07-22T16:45:26Z

src/view/ThemeManager.js

@@ -87,7 +87,7 @@ define(function (require, exports, module) {
        this.file        = file;
        this.name        = options.name  || (options.title || fileName).toLocaleLowerCase().replace(/[\W]/g, '-');
        this.displayName = options.title || toDisplayName(fileName);
-        this.dark        = options.theme && options.theme.dark === true;
+        this.dark        = !!options.theme.dark;


is it possible that options.theme could ever be undefined? This will give an error. In fact, it does in the tests (which don't run now).

MiguelCastillo · 2014-07-22T17:31:07Z

@dangoor, closing this one in favor of #8505. That has a normal history line

mackenza and others added 12 commits July 17, 2014 14:19

refactored fix from feedback

2b0492a

changed else if to else

95de952

fixed fat fingered else error... thx travis

61538d2

small typo in comment

9660850

removed else condition based on review comments

fbafd76

Negative areas of the selection triangle need to be transparent.

5b653f8

Need this for the color values in inline color picker.

37fe666

Merge pull request #8439 from mackenza/issue-8395

ae17456

Recognize existing indentation when indenting close braces

Merge branch 'master' of https://github.com/adobe/brackets into larz/…

9cc83ba

…missed-a-spot

It's scary to think that the loading order of cm-* selectors matter.

d20f504

Fix for #8491.

d46fcd8

Logic to make sure scrollbar processing only happens in the editor sp…

16aeac1

…ace. Also fixed issue with scrollbar regex. #8482

Fix for #8494.

92b07c8

Larz and others added 3 commits July 21, 2014 22:34

Missed overwrite styling. Fixed based on @TomMalbran's feedback.

6166655

Merge pull request #8484 from adobe/larz/missed-a-spot

d6cf64f

Make negative areas of selection triangle transparent (& other fixes)

Adding regexp to jslint whitelist

d01b028

dangoor added 2 commits July 22, 2014 11:19

Merge pull request #8492 from MiguelCastillo/fonts

93bea99

Fonts api with Themes integrated

Adds dark class and moves inline-widget color into core UI theme

2beb386

dangoor reviewed Jul 22, 2014
View reviewed changes

MiguelCastillo added 3 commits July 22, 2014 12:00

Logic to make sure scrollbar processing only happens in the editor sp…

b88df9f

…ace. Also fixed issue with scrollbar regex. #8482

Adding regexp to jslint whitelist

0d6a11a

Merge remote-tracking branch 'upstream/dangoor/dark-class' into theme…

a2de718

…s-8482

Changed dark class to be added to body rather then anywhere platform …

7f32e9b

…is found. Also added coercion to make theme.dark always be a boolean

Merge branch 'themes-8482' of https://github.com/MiguelCastillo/brackets

c19a4f9

into themes-8482 Conflicts: src/view/ThemeManager.js

dangoor reviewed Jul 22, 2014
View reviewed changes

MiguelCastillo mentioned this pull request Jul 22, 2014

Themes scrollbars #8505

Merged

dangoor closed this Jul 22, 2014

MiguelCastillo deleted the themes-8482 branch July 22, 2014 21:15

This was referenced Aug 30, 2021

[CLOSED] Logic to make sure scrollbar processing only happens in the editor space... brackets-archive/bracketsIssues#7592

Open

[CLOSED] Themes scrollbars brackets-archive/bracketsIssues#7603

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Logic to make sure scrollbar processing only happens in the editor space... #8493

Logic to make sure scrollbar processing only happens in the editor space... #8493

MiguelCastillo commented Jul 22, 2014

MiguelCastillo commented Jul 22, 2014

MiguelCastillo commented Jul 22, 2014

TomMalbran commented Jul 22, 2014

peterflynn commented Jul 22, 2014

MiguelCastillo commented Jul 22, 2014

dangoor Jul 22, 2014

MiguelCastillo Jul 22, 2014

dangoor Jul 22, 2014

MiguelCastillo Jul 22, 2014

dangoor Jul 22, 2014

marcelgerber Jul 22, 2014

MiguelCastillo Jul 22, 2014

MiguelCastillo Jul 22, 2014

dangoor commented Jul 22, 2014

dangoor commented Jul 22, 2014

dangoor Jul 22, 2014

MiguelCastillo commented Jul 22, 2014

		.then(function (cssContent) {
		$("[class\|=platform]").toggleClass("dark", theme.dark);

Logic to make sure scrollbar processing only happens in the editor space... #8493

Logic to make sure scrollbar processing only happens in the editor space... #8493

Conversation

MiguelCastillo commented Jul 22, 2014

MiguelCastillo commented Jul 22, 2014

MiguelCastillo commented Jul 22, 2014

TomMalbran commented Jul 22, 2014

peterflynn commented Jul 22, 2014

MiguelCastillo commented Jul 22, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dangoor commented Jul 22, 2014

dangoor commented Jul 22, 2014

Choose a reason for hiding this comment

MiguelCastillo commented Jul 22, 2014