Skip to content
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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[kbn/optimizer] Should we build both light and dark themes by default? #70670

Open
spalger opened this issue Jul 2, 2020 · 8 comments
Open
Labels

Comments

@spalger
Copy link
Contributor

spalger commented Jul 2, 2020

With #70389 the KBN_OPTIMIZER_THEMES environment variable can now be used to customize the themes that the @kbn/optimizer converts SASS imports into. By default #70389 builds the v7light and v7dark themes and provides a 20% performance improvement.

For devs who are only working in a single theme most of the time performance can be improved an additional 20% by choosing to build a single theme like so:

KBN_OPTIMIZER_THEMES=v7light yarn start

Additional details about this environment variable are available in the @kbn/optimizer readme. For additional details about how themes are loaded and how errors are surfaced when a mismatch occurs checkout the description of #70389

What should the default be?

We chose to move forward with the default KBN_OPTIMIZER_THEMES value of v7light,v7dark because it is assumed that devs regularly need to switch between light and dark mode, or at last should be, and hiding that ability behind an environment variable would be a worse experience than a 20% performance boost.

Please leave us a note about what you wish the default was, and perhaps some context about how often you use dark mode while developing in Kibana.

@spalger spalger added the discuss label Jul 2, 2020
@snide
Copy link
Contributor

snide commented Jul 2, 2020

I'd prefer the default to be both light and dark themes, since that's our current support. To me this is equivalent of defaulting to loading all plugins. We do it so that's checked regularly.

It's awesome that we can turn it to just one and cut down load time, and we can just message that broadly. I'm sure a lot of folks never touch the sass layer and have a subjective preference for only one thing or the other. For those folks, the speed is a huge win.

My reasoning for keeping the default is that we have no real testing around theming, and I don't think it likely we ever will in any sort of automated fashion. The more checking we can do there the better.

If we DO decide to only load only one of the themes by default for local dev, it's no big deal, but I'd actually prefer it be set to dark mode. It's where we have issues, and that might be enough to spur folks to be more aware of it.

@dakrone
Copy link
Member

dakrone commented Jul 2, 2020

I think it would be a really poor experience to remove any theme from the optimizer. I also think it's a common misconception that "devs want dark mode", I know some do, but I don't think we should assume it.

Also, performance changing based on what theme you use sounds really terrible, we would be mocked (and rightly so!) in many blog posts to come if we made that change.

@spalger
Copy link
Contributor Author

spalger commented Jul 2, 2020

@dakrone to be clear, I'm describing the themes that would be available in the default developer environment. This doesn't lead to any product changes, and should we choose to change the default to only build one theme it would most likely be the current default theme, k7light. @snide makes a good point about dark mode needing more attention, but I suspect we'd have the opposite problem if we defaulted to dark mode.

@dakrone
Copy link
Member

dakrone commented Jul 2, 2020

@dakrone to be clear, I'm describing the themes that would be available in the default developer environment. This doesn't lead to any product changes.

Oh hah okay, well you can ignore me then 😄

I do agree however, that we should endeavor to treat themes identically anywhere though.

@clintandrewhall
Copy link
Contributor

clintandrewhall commented Jul 3, 2020

Perhaps we could introduce yarn start-dev that could start Kibana with not just this optimization, but any others we devise... it could list a set of env variables and what they're set to... then it's clear you're starting with optimizations you can disable, (and which you could disable), by setting them in command prompt?

/kibana: yarn start-dev

You've started Kibana in dev mode.  The following variables are set for a faster experience:
KBN_OPTIMIZER_THEMES=v7light  - only light theme is enabled
KBN_OPTIMIZER_FOO=bar         - only one base belong to us

...

Consider any command: yarn start-opt, yarn start-thin, etc... or even just yarn dev

@cjcenizal
Copy link
Contributor

cjcenizal commented Jul 6, 2020

Big +1 to @clintandrewhall's point. This is exactly the suggestion I wanted to make when I came to this issue. The slow optimization time is THE big DX killer. It wrecks our iteration speed and when you multiply it by all of our engineers, you get a huge amount of wasted time and interrupted flows. I'd like to see us accumulate small hacks and wins like this. I'd just suggest that each dev should be able to specify some custom settings (e.g. default to dark mode) for when they use the yarn start-dev command, so they don't need to keep re-selecting them. Maybe these could be in some file, similar to how the backport tool is configured?

To @snide's point about testing dark mode, I agree. I just hope we can find a way to encourage this while also supporting improved iteration speed. For example, the ES UI team has added "Test UI in dark mode" to our roadmap issue template.

@paul-tavares
Copy link
Contributor

@spalger the 20% improvement is only on start and not evertime the optimizer runs, correct? (unless you are making changes to the theme, then I understand it may be every time it builds).

@spalger
Copy link
Contributor Author

spalger commented Jul 31, 2020

@paul-tavares correct

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants