New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Theming in Atom #4

Open
wants to merge 3 commits into
base: master
from

Conversation

Projects
None yet
4 participants
@simurai
Member

simurai commented Jan 2, 2018

馃悪 Project Mimicus

Question: Should Atom simplify theming?

馃憠 Rendered

Tests

  • Move UI theme's styles to bundled packages
    • Issues found: Will break UI themes, maybe certain packages
    • Decision: 馃憤 UI themes can still override anything, just adds more maintenance costs.
  • Use custom properties over Less variables
    • Issues found:
      • Can't use color functions.
      • Can't use when.
    • Decision: 馃憤 In practice Less variables were not needed as often as feared. As a workaround for when, JS might can be used.
  • Let core and language packages do the syntax highlighting
    • Issues found: Breaks Syntax themes.
    • Decision: 馃憤 Basic migration should be quite easy. And to keep Syntax themes unchanged, they could override core/languages if really needed.
  • Overhaul and extend UI library
    • Issues found: Class names/markup changes, thus breaks packages
    • Decision: 馃憤 We might can keep some of the old styles for a while until most packages are updated.
  • Use BEM like naming convention for classes: #5
    • Issues found:
      • Breaks all themes and some packages. Keep in mind that CSS classes are used to scope keybindings. So even if a package has no visible UI, the keybindings or context menu may still break.
      • Takes the most effort. If we already make breaking changes, then it's tempting to also do some refactoring, improve UX or anything we kept holding off.
      • This might introduce merge conflicts with current PRs.
    • Decision: 馃 I think we should still do it, but a lot will break and it's a big effort.

Affected packages

Note: This is just a guess, there might be more. Plus all the community packages.

  • atom/atom 馃憠 changes
  • atom-ui 馃憠 changes
  • atom-theme-temp 馃憠 changes (only used temporarily, use as UI theme)
  • github 馃憠 changes
  • atom-ide-ui
  • teletype
  • atom-select-list
  • atom-dark-syntax
  • atom-dark-ui
  • atom-light-syntax
  • atom-light-ui
  • base16-tomorrow-dark-theme
  • base16-tomorrow-light-theme
  • one-dark-ui
  • one-light-ui
  • one-dark-syntax
  • one-light-syntax
  • solarized-dark-syntax
  • solarized-light-syntax
  • about
  • archive-view
  • autocomplete-plus
  • background-tips
  • bookmarks
  • bracket-matcher
  • command-palette
  • deprecation-cop
  • exception-reporting
  • find-and-replace
  • fuzzy-finder
  • git-diff
  • image-view 馃憠 changes
  • incompatible-packages
  • keybinding-resolver
  • markdown-preview
  • notifications
  • package-generator
  • settings-view
  • status-bar 馃憠 changes
  • styleguide 馃憠 changes
  • tabs 馃憠 changes
  • timecop
  • tree-view
  • welcome
  • wrap-guide

Decision

TBD


Name inspiration: https://www.youtube.com/watch?v=os6HD-sCRn8

@nathansobo

This comment has been minimized.

nathansobo commented Jan 2, 2018

I'm really happy to see you exploring this, and I like the direction. Some thoughts in reaction to this (not super well organized).

  • I'd love to see us in a position to deprecate Less some time this year so that we could potentially purge it from the system and reduce complexity. It seems like CSS variables could be part of the key.
  • We could potentially introduce a much more limited form of CSS pre-processing to enable the use of vendor-prefixed color adjustment functions that compile down to calc. For example, you could write background: -atom-darken(var(--main-bg-color), .2) and that would get macro-expanded to the equivalent function implemented with calc. This could have its downsides in terms of long-term maintenance burden and complexity, but it could be an option for making it reasonable to achieve advanced effects in a flexible way.
  • I really like the idea of simplifying syntax highlighting to a set of simple variables. It's not clear to me however how important structural styling is, where styling is specific to a variable in a specific context. I know it's something that TextMate themes always enabled. I suspect 90% of the time or more it doesn't matter. Ultimately, if we could get away with steering the community toward a set of straightforward variables in a flat namespace, it could really help our performance efforts by giving us more options in terms of how the text is rendered. We could reduce the complexity of each line by requiring little to no nesting, or potentially render everything with a canvas and bypass the DOM entirely.
  • Style recalculation is a big bottleneck when scrolling the editor, and simplifying our selectors could improve that dramatically. But as you know it's a big job that has to be done across the entire application and ecosystem.

I'm really glad you're looking at this. I think it's incredibly important for the long term health of Atom that we iterate our approach to theming and address the existing issues. It's a big project, but it's going to be great to get it started.

@simurai

This comment has been minimized.

Member

simurai commented Jan 4, 2018

background: -atom-darken(var(--main-bg-color), .2)

That would be cool, but not sure about performance impact. Because if --main-bg-color gets changed at runtime, all the styles would need to be re-calculated. Which we tried with Less, but it was too slow.

Anyways, I started with https://github.com/atom/atom-theme/blob/master/index.css. Let's see how often color functions are needed.

@nathansobo

This comment has been minimized.

nathansobo commented Jan 4, 2018

Which we tried with Less, but it was too slow.

But Less is a preprocessor right? So in that case we had to preprocess everything, whereas these custom transformation functions would compile down to calls to calc in the actual CSS.

What I hadn't realized until looking at your link is that there's a built-in hsl function. It seems like that could cover a ton of ground. I don't know enough about color transformation to know its limits, but obviously you can easily tweak the saturation and brightness of a base hue, which seems like most of what you'd need to do. Great news.

@simurai

This comment has been minimized.

Member

simurai commented Jan 5, 2018

But Less is a preprocessor right? So in that case we had to preprocess everything, whereas these custom transformation functions would compile down to calls to calc in the actual CSS.

Ohh.. you mean it could be a custom function implemented with Houdini? Yeah, sounds like that will be possible one day. But my guess is that CSS color functions might be available before since there is already a spec for it?

there's a built-in hsl function.

Yeah, maybe for the most common colors, like the editor background, we could split it into hue, saturation and lightness properties. And then you can stitch them together in hsl and use calc:

:root {
  --bg-h: 208;
  --bg-s: 66%;
  --bg-l: 99%;
  --bg: hsl( var(--bg-h), var(--bg-s), var(--bg-l) ); /* default bg */

  --my-bg: hsl( var(--bg-h), var(--bg-s), calc( var(--bg-l) - 20%) ); /* darken by 20% */
}

Doing that for every color used in a theme might be overkill, but should be helpfull for the most common colors used.

@nathansobo

This comment has been minimized.

nathansobo commented Jan 5, 2018

Ohh.. you mean it could be a custom function implemented with Houdini?

Basically I'm talking about a macro language that compiles down to stuff that would use standard features like variables and calc at the actual runtime.

Your example is a great showcase of how it would work. Everything you wrote works today, but it's really verbose to write. So the preprocessing would just be designed to make it easier... So you could write something like --bg: -atom-hsl(207, 66%, 99%) and it would automatically define --bg-h, --bg-s, and --bg-l. Then you could write --my-bg: -atom-darken(var(--bg), 20%) and that would automatically expand to --my-bg: hsl( var(--bg-h), var(--bg-s), calc( var(--bg-l) - 20%)) in the actual rendered CSS.

I'm not saying this is necessarily a great idea, and could lead us down the road of maintaining non-standard stuff. But I just wanted to clarify what I'm trying to express.

@Alhadis

This comment has been minimized.

Alhadis commented Jan 5, 2018

Wouldn't snippets be a better solution for the verbosity...?

I have a CSS (and Less) snippet that expands . into .syntax-- whenever I add a rule which targets syntax highlighting, for example. Works perfectly.

@nathansobo

This comment has been minimized.

nathansobo commented Jan 5, 2018

Yeah, it's probably better to just eat the verbosity for the sake of just using standard features. The code you own ends up owning you. It was just an idea I wanted to put out there.

@simurai

This comment has been minimized.

Member

simurai commented Jan 11, 2018

Tried to simulate color functions in this test: https://codepen.io/simurai/pen/RxxzoG/?editors=0100. Kinda works, but still somewhat limiting, especially when trying to get enough contrast for the text labels.

Another concern about the verbosity: It's not just having to type a lot, it's also reading and understand how it works. Theme/package authors might get too overwhelmed and would first have to read a blog post or the docs? So for the default theme, maybe just having simple hsl colors might be easiest to understand. Like:

:root {
  --fg:        hsl(180, 20%, 80%);
  --fg-strong: hsl(180, 50%, 99%);
  --fg-subtle: hsl(180, 10%, 50%);
}

Theme authors can always split it up more and use calc for easier authoring, just not the default, which also serves as a template.

So far, the only time I needed separate hsl channels is for the cursor-line. Because it needs to be semi transparent to not cover highlights underneath. We'll see if there are more occasions.

@simurai

This comment has been minimized.

Member

simurai commented Jan 11, 2018

Something neat: Have colors automatically adapt when inside a certain area.

For example when a dock item moves to the center, it would use the colors for the center area (green in the example below):

regions

Theme authors would have to define some variables for each area, currently there are 3:

  1. App (docks, status-bar etc.)
  2. Editor (center panes)
  3. Overlays (command panel, auto-complete etc.)
@nathansobo

This comment has been minimized.

nathansobo commented Jan 11, 2018

Adaptive colors are pretty dope.

@simurai

This comment has been minimized.

Member

simurai commented Jan 12, 2018

After testing status-bar I moved on to the tabs and already hitting a roadblock. 馃檲

  1. If I remove the list-inline class,
  2. this spec fails.
  3. But it keeps failing even if I copy the styles from that class over to tabs.less.

It's so bizarre. As if the spec relies on the atom-ui styles, but the styles from the package have no influence? 馃

Anyways, maybe I'll leave the tabs for now and test some other packages to see if stuff like this happen again.

@nathansobo

This comment has been minimized.

nathansobo commented Jan 12, 2018

@simurai It looks like that's testing auto-scrolling to a tab when it's activated. Did you confirm that this actually breaks or is it just an issue with the test? It seems like we should be able to figure out how to get it working again.

simurai added some commits Jan 18, 2018

@simurai simurai referenced this pull request Jan 22, 2018

Open

CSS Naming Convention #5

@matthewwithanm

This comment has been minimized.

Member

matthewwithanm commented Feb 14, 2018

Something neat: Have colors automatically adapt when inside a certain area.

馃憤 I wanted this for a theme a while back but abandoned the idea because of the fragility of trying it with the current system. We still do contextual styling with the console but this would be nicer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment