-
Notifications
You must be signed in to change notification settings - Fork 7.6k
Conversation
…css-refactor Conflicts: src/styles/brackets.less
@TomMalbran good idea! Forgot about that. |
To see the Dark UI:
|
@@ -156,7 +156,7 @@ define(function (require, exports, module) { | |||
* @type {FileUtils.LINE_ENDINGS_CRLF|FileUtils.LINE_ENDINGS_LF} | |||
*/ | |||
Document.prototype._lineEndings = null; | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These files where the only changes are whitespace should not be part of the PR. It's unnecessary noise for reviewer and also for the file history.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Damn I think I have an extension that strips those out automatically. Do I need to add the whitespaces back?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@redmunds For review, you can use this URL: https://github.com/adobe/brackets/pull/8362/files?w=0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need to sort out the issues in #8420 before we can land this. I'm not entirely clear on how the color overriding works here, but at first glance it looks like it would suffer from the same problem. But also I think we intended to punt this work to a future sprint, as part of https://trello.com/c/nhQxRyWn/518-theme-support-for-light-dark-mode -- and just focus on fixing #8379 for this release. |
Ok sounds good. |
@larz0 It sounds like your schedule is pretty full today. I can take a crack at converting what you've done here to work with the new theme style. |
Cool thanks @dangoor. |
@larz0 I'm going to close this since we know we can't implement the dark colors using this approach. A lot of this has been ported to #8529 already, and I assume next sprint we can continue that approach to capture the rest of what's posted here (panel colors, etc.). As long as we don't delete the branch yet, we can still look at the diff here for reference: master...larz/css-refactor |
@peterflynn ok sounds good as long as we're keeping this branch for reference. |
@larz0 @peterflynn What about having 2 less main files: In
In
When switching themes with a different dark option we can switch the files. Also when compiling less we will get 2 files |
@TomMalbran Exactly. See https://trello.com/c/nhQxRyWn/518-research-theming-improvements-maintainability for more discussion. |
@peterflynn Nice. I didn't saw that card. Anyway we shouldn't need any script to generate the files. We should only need manually create the files I mentioned and update the Gruntfile so that it compiles 2 less files instead of 1. We can even do something similar for extensions. They give the less file with the variables and we compile it. This can be done at the registry level or at runtime. |
To see the Dark UI:
brackets_core_ui_variables_dark.less
inbrackets_shared.less
.brackets_core_ui_variables_dark.less
into the appropriate core extension stylesheets.