search

brackets-archive / bracketsIssues

Archive of issues in brackets.
0 stars 0 forks source link

[CLOSED] apply theme to .force-syntax-highlighting #9248

Open core-ai-bot opened 3 years ago

core-ai-bot commented 3 years ago

Issue by mat-mcloughlin Monday Jan 26, 2015 at 16:50 GMT Originally opened as https://github.com/adobe/brackets/pull/10468

I've added changed where the theme gets applied so it also applies to any element with the .force-syntax-highlighting class

mat-mcloughlin included the following code: https://github.com/adobe/brackets/pull/10468/commits

core-ai-bot commented 3 years ago

Comment by MiguelCastillo Monday Jan 26, 2015 at 17:19 GMT

@mat-mcloughlin I like the idea. Initially the rules were not restricted to just the editor, then we restricted them to the editor because we didn't want to allow theme authors to mess with the rest of the application because the DOM could change and then themes could potentially be broken.

But I like the opt-in approach. @peterflynn , do you have any objections to this change?

core-ai-bot commented 3 years ago

Comment by mat-mcloughlin Monday Jan 26, 2015 at 17:35 GMT

You'll be making me happy if you do :)

image

core-ai-bot commented 3 years ago

Comment by MarcelGerber Sunday Feb 01, 2015 at 13:40 GMT

Btw, this was discussed in https://groups.google.com/forum/#!topic/brackets-dev/6_TFeUSUf80.

core-ai-bot commented 3 years ago

Comment by MiguelCastillo Sunday Feb 01, 2015 at 14:23 GMT

Oh thanks for the link!

@mat-mcloughlin I think the use case for this class goes beyond the scope of syntax highlighting, so we should come up with a more generic name. Meaning that adding this class will allow somebody to style the tree, for example. If this is only for syntax highlighting, why do you need to rely on themes though? Couldn't you have your extension load its own css/less and add styling that way? Check out how I do it in ternific - here. You can load up your css file like this, for example.

Let me know what you think.

core-ai-bot commented 3 years ago

Comment by MiguelCastillo Sunday Feb 01, 2015 at 14:36 GMT

Thinking about it a bit more, this needs to be thought out a bit more. This actually falls in the theming for the whole tool, which we have a trello card for. This quick one off patch is most likely going to be obsoleted. I don't think we should add this - I don't know how whole editor theming is going to play out.

@mat-mcloughlin I really suggest adding your own style as I described in my previous post. Let me know if I can help you there.

core-ai-bot commented 3 years ago

Comment by MarcelGerber Sunday Feb 01, 2015 at 16:14 GMT

I still think it's a good idea. He tries to match the appearence of his code hints popup with how it'll look like after being inserted into the editor. Also, I wouldn't call it whole-tool themes/schemes/WhateverYouWannaCallIt. In this case, the extension explicitly asks to be styled using themes, while whole-tool themes would do it the other way round, they'd actively apply their styles to the whole Brackets UI.

core-ai-bot commented 3 years ago

Comment by mat-mcloughlin Sunday Feb 01, 2015 at 16:37 GMT

Hey, I'll take a look but the point of this is that it would apply the users chosen theme to the code hint popup.

core-ai-bot commented 3 years ago

Comment by MiguelCastillo Friday Mar 27, 2015 at 00:16 GMT

@mat-mcloughlin did you end up resolving your issue?

core-ai-bot commented 3 years ago

Comment by mat-mcloughlin Friday Mar 27, 2015 at 09:37 GMT

No I ended up leaving it. Its a nice to have and didn't want to implement a complicated solution until I knew what you were planning regrading implementing this in to core

core-ai-bot commented 3 years ago

Comment by MiguelCastillo Friday Mar 27, 2015 at 12:10 GMT

Fair enough. I agree there is value in having this non-intrusive solution that most likely will not interfere with any further work on theming.

core-ai-bot commented 3 years ago

Comment by MiguelCastillo Friday Mar 27, 2015 at 12:18 GMT

@dangoor@redmunds are you guys ok with this minor tweak to enable folks to accomplish their goals more easily? I think this adds value.

The caveat of course is that any future DOM changes outside of #editor-holder have a chance of breaking extensions, so we would want to add a h1-bolded disclaimer on this. This is the only reason I have been a bit hesitant about it.

core-ai-bot commented 3 years ago

Comment by dangoor Friday Mar 27, 2015 at 13:01 GMT

Since this is opt-in, it seems reasonable to me.

core-ai-bot commented 3 years ago

Comment by mat-mcloughlin Friday Mar 27, 2015 at 13:54 GMT

so should I change it to theme-override?

core-ai-bot commented 3 years ago

Comment by MiguelCastillo Friday Mar 27, 2015 at 14:01 GMT

theme-override or another name that is generic... If there are no options to chose from, then we can just go with theme-override.

core-ai-bot commented 3 years ago

Comment by redmunds Friday Mar 27, 2015 at 14:31 GMT

Looks good to me.

The caveat of course is that any future DOM changes outside of #editor-holder have a chance of breaking extensions

I don't see how this change has any effect on future DOM changes breaking extensions. Let me know if I'm missing something.

core-ai-bot commented 3 years ago

Comment by redmunds Friday Mar 27, 2015 at 14:58 GMT

How about .apply-editor-coloring, .apply-editor-syntax-coloring, or .editor-syntax-coloring ?

core-ai-bot commented 3 years ago

Comment by MiguelCastillo Friday Mar 27, 2015 at 15:12 GMT

@redmunds if someone writes an extension that for example styles some part of the file tree using this new class... And the file tree goes through a change in which a class is renamed/removed or a div is added, then such extension could break because they are writing css rules that target something that changed.

Also, the class name could be more generic. Using that class will enable someone to theme the file tree. At which point .apply-editor-coloring is no longer accurate.

core-ai-bot commented 3 years ago

Comment by redmunds Friday Mar 27, 2015 at 15:28 GMT

@MiguelCastillo

if someone writes an extension that for example styles some part of the file tree using this new class... And the file tree goes through a change in which a class is renamed/removed or a div is added, then such extension could break because they are writing css rules that target something that changed.

That's the same as it's always been. I guess the point is that by providing this mechanism, we're endorsing these types of extensions so it will be more likely. As you said, we should have a disclaimer that we don't guarantee that this will never change and extension authors should watch for API changes.

core-ai-bot commented 3 years ago

Comment by MiguelCastillo Friday Mar 27, 2015 at 15:33 GMT

@redmunds yeah exactly :) endorsing is the key.

core-ai-bot commented 3 years ago

Comment by mat-mcloughlin Friday Mar 27, 2015 at 15:47 GMT

@redmunds@MiguelCastillo is that a yea or nay :)

core-ai-bot commented 3 years ago

Comment by MiguelCastillo Friday Mar 27, 2015 at 15:48 GMT

@mat-mcloughlin its a yea :) just brining everyone up to speed on the implications.

core-ai-bot commented 3 years ago

Comment by abose Friday Feb 12, 2016 at 06:30 GMT

Tagging@swmitra