Open carlosms opened 5 years ago
Thanks @carlosms @smacker for this issue. Other than what you described, I don't have that much to add. The variables defined in custom-brand.less
get overwritten somehow. If I redefine @font-size-base:
for instance, it gets overwritten and I have to declare it on each element.
not a prio for next R6-CE, will resume next week
The variables defined in
custom-brand.less
get overwritten somehow. If I redefine@font-size-base:
for instance, it gets overwritten and I have to declare it on each element. Variables in less are lazily defined, and we have:
superset.less <<-- entrypoint
@imports index.less
@imports vars.less
index.less
@imports vars.less
@imports override.less
vars.less
@brandColor: red
override.less
@brandColor: blue
body {
color: @brandColor
}
When superset.less
is compiled into superset.css
, it will appear as it follows:
what would be equivalent to:
superset.less
@brandColor: red
@brandColor: blue
body {
color: @brandColor
}
@brandColor: red
so since vars are defined twice (the second one at the end), that one wins, so no override is done:
superset.css
body {
color: red
}
Here is my diagnostic considering our goal of applying our branding over Apache Superset. I also added some recommendations that I'd like to address in order to enhance the developer experience when rebranding Apache Superset.
note: all routes are relative to superset/superset/assets/
In Apache Superset there are many ways to define the APP styles; they can be summarized in two:
GLOBAL
: src/theme.js
, which calls stylesheets/superset.less
COMPONENTS
: each view, component or element, has its own .jsx
which loads its own less
or css
current limitations:
COMPONENTS
are applied after GLOBAL
.About how dimensions, colors, and typographies are defined, we have four situations, that are detailed in each collapsible section below.
TL;DR As it can be seen from the list below, to obtain style consistency, it may be needed either overriding many rules defined by stylesheets that are not using shared variables at all, either modifying those stylesheets to use shared variables.
There are two (independent) files defining variables and are used sometimes in some stylesheets
But there are many stylesheets that do not use variables at all:
MAIN
stylesheets/less/cosmo/variables.less
into the source of shared variablesCUSTOM
variables stylesheet stylesheets/less/cosmo/variables-override.less
.MAIN
variables stylesheet imports our new CUSTOM
variables stylesheet to override MAIN
values.
MAIN
variables stylesheet in our custom stylesheet, instead of our CUSTOM
one.The ones above can be done easily by us; the next ones could be done gradually (and are ordered from costless to very hardcore):
CUSTOM
stylesheet, stylesheets/less/custom-brand.less
as a complete separate stylesheet at the end of the page (after COMPONENTS
stylesheets, instead of before MAIN
one). Doing so:
!important
hacks, or playing with CSS specificity.src/dashboard/stylesheets/variables.less
to use the variables from MAIN
, so the rest of the components which are already using this other, will actually be using our MAIN
variables.
MAIN
and in src/dashboard/stylesheets/variables.less
, even when they share the same name, so they should be considered separately.we could now continue considering the recommendations above
@dpordomingo what's the status of this? is this waiting for product's approval? is this done and we can open a separate issue for next steps?
I'd consider this as an umbrella taking into account my analysis and my recommendations.
I'm also aware that @ricardobaeta has a pending investigation about how could we integrate into superset
some of the improvements that we did, and that would be useful if they were already in upstream.
Alternatives that I see:
I'd prefer to close this one and if you could create a new one with a more specific scope according to your recommendations. 👍 If you do, please close this one.
I'd say this should be considered an epic, taking my message describing the current status as an input, and my recommendations as one proposed strategy to solve the problem in an iterative way.
Would you prefer if I copy-paste both into a new issue and close this? Or maybe link both in the issue desc, to have the whole history here, and clear details from the very beginning of the thread?
Maybe the best thing would be to create a new issue as a story in src-d/applications-backlog
and then from that we would create all the different subtasks? wdyt? Similarly to what we did for the log level thing.
Since src-d/applications-backlog
is a private repo, it would hide all this info, which is something that I dislike because this info is pretty valuable considering sourced-ui
as open-source from apache/incubator-superset
.
But if you prefer, I can still do it.
Oh right, sorry, I forgot that it's private. I just thought about it because it's the place where we now put stories. Well, this is something that actually affects all the projects. I'm pretty sure that @rpau already answered me, and that I forgot, but @rpau was there a specific reason for keeping it private?
I agree with moving there, things that are not open source related, like the ones affecting company products, business, strategic decisions... But I'd say this is not the case.
Well, originally it was supposed also to be a collector of issues for external users AFAIR. Let's keep it in sourced-ui then for now 👍
This issue is to improve LESS code to make use of variables. @ricardobaeta please add any other things that need to be reviewed, or that you need assistance with.