You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
in order to support theming across all solutions using expressions we should align on what a theme is, how its used and a set of functions to work with it.
Charts should have arguments allowing to set styling properties like font, palette, line options. They should all use the same context type for those argument (font for font, palette for color palette, ….). For each of those types we provide a set of functions to generate those types.
Lets take a look at example pie function which has two arguments font and palette:
font function can generate font context type. It has a bunch of arguments, which all have defaults
args: {
align: {
default: 'left',
help: i18n.translate('expressions.functions.font.args.alignHelpText', {
defaultMessage: 'The horizontal text alignment.',
}),
options: Object.values(TextAlignment),
types: ['string'],
},
color: {
help: i18n.translate('expressions.functions.font.args.colorHelpText', {
defaultMessage: 'The text color.',
}),
types: ['string'],
},
family: {
default: `"${openSans.value}"`,
help: i18n.translate('expressions.functions.font.args.familyHelpText', {
defaultMessage: 'An acceptable {css} web font string',
values: {
css: 'CSS',
},
}),
types: ['string'],
},
Proposal: We change those defaults to {theme 'font.family'} where theme function retrieves the key provided from a theme service. A property to this function can also be a name to select which theme you want to use (in case we would want to override selected theme on per-expression basis) and default to provide default value in case theme information is not available.
If expression function (chart) wants a different default than just {font} (wants to define the default font to something else than what font function defines) there are two options:
- Chart don't want the theme information to override this:
Set default to {font family='arial'}
- Chart would still want the theme info to override this if its present
Set the default to {font family={theme 'font.family' default='arial'}}
Chart functions don't need to know anything about themes, they just know about their styling properties. Only functions to generate the styling context types know about themes and only theme function has access to theming service.
If we were to implement different theming service in canvas vs dashboard, canvas would only need to override theme function, everything else would stay the same (not that we should actually do that, we should go with one theming implementation imo)
Allows us to implement themes for any visual property (actually for every argument), allows us to introduce global overrides at a later point as well as user overrides.
No need to migrate anything. Canvas functions can stay exactly the same. Canvas stored expressions can stay exactly the same, lens expressions can stay exactly the same. (we'll take a look at this under examples)
Support for embeddables comes almost for free (we'll take a look at this under variables)
Examples:
Lets take a look at some expressions, and how would they look after interpreter parses them and inserts default arguments.
A simple pie expression with all the arguments defaulted
pie
pie font={font} palette={palette} interpreter inserted default arguments
pie font={font family={theme 'font.family'} size={theme 'font.size'}} palette={palette name={theme 'palette.name'}}}
interpreter inserted default arguments for font and palette functions as well
we can see that a stored expression with pie function where styling arguments were defaulted (not set) will suddenly start supporting the themes
A pie expression with some arguments set (not-defaulted)
pie font={font size=26}
pie font={font size=26 family={theme 'font.family'}} palette={palette name={theme 'palette.name'}}
interpreter sets the defaults for the missing arguments (defaulted), for which theming will work,
for arguments that were overriden the theming won't override them (expected behaviour)
Setting a different default on a chart function then the default of styling function is
pie font={font family={theme 'font.family' default='arial'}} if theme 'font.family' is not set use arial default
Implemented with variables:
pie font={font family={theme default='arial' } size={theme 'font.size'}} pie font={font family={var 'theme.font.family' default='arial' } size={var 'theme.font.size'}
Everything needed to implement this is already in place, steps we need to take to make this a reality:
- agree on theming variables we'll use
- change default values of styling functions to use the agreed on variables:
- For example: font function default for size is 14. we need to change it to var 'theme.font.size' default=14 which will give us same result if theme info is not present,
-> but if present it will use theme information
- Give variables to executor (and build UI around them)
- Add variables to embeddable base input (to make it work with embeddables directly (inside and outside of canvas))
Concerns:
- We loose type safety (as we anyway do with variables. Variable can be of any type so var function can return anything and we can't check that at compile time but only at runtime)
- Could we have default/forced theme variable for specific arguments ? To avoid using wrong variables for wrong arguments
- We should have theme variables as a separate high level concept to prevent colisions
Proposal 2:
Similar to proposal 1, but to solve type safety concert and concert with having wrong variables specified: instead of having a special theme function each of the styling functions should work directly with the theming service. Styling functions should have no default arguments defined on the expression function definition, but rather have the defaults asigned inside the function when the argument is undefined.
Benefits:
Chart functions don't need to change but styling functions do. However each styling function needs access to the theming service.
Implementing of user overrides or global overrides might be more complicated as we'll need to do that in several styling functions (vs single function in proposal 1), but we might make that easier providing a set of utility functions each styling function would then use. (like getThemeProperty('font.family', 'arial') and that function internally can check the theme, global overrides, user overrides)
We still don't need to migrate anything. No arguments to any functions would be changed, just styling function implementations would know that when there is no argument provided they should check with theming service for availability of it.
Support for embeddables still comes almost for free if we would be using variables for passing down theme information. Another option would be a global theming service (which imo should be avoided).
We avoid 1st and 2nd concert of proposal 1 with the downside of multiple functions needing access to theming service and implementations of those styling functions will be more complicated.
Examples: pie pie font={font} // interpreter inserts default argument, however font doesn't have defaults set for any of its arguments
// Internally font uses a getThemeProperty function when arguments were not given and provide default for when theme is not available
TLDR:
No matter which of above proposals we would go with we first need to agree on context types for our visual attributes (font, palette, ....), provide base set of functions to work with those context types and have all charting functions exposing arguments allowing to set this. Once that is in place we should be able to implement theming without breaking backward compatibility or needing migrations.
The text was updated successfully, but these errors were encountered:
in order to support theming across all solutions using expressions we should align on what a theme is, how its used and a set of functions to work with it.
Charts should have arguments allowing to set styling properties like font, palette, line options. They should all use the same context type for those argument (
font
for font,palette
for color palette, ….). For each of those types we provide a set of functions to generate those types.Lets take a look at example
pie
function which has two argumentsfont
andpalette
:font
function can generatefont
context type. It has a bunch of arguments, which all have defaultsProposal: We change those defaults to
{theme 'font.family'}
where theme function retrieves the key provided from a theme service. A property to this function can also be aname
to select which theme you want to use (in case we would want to override selected theme on per-expression basis) anddefault
to provide default value in case theme information is not available.If expression function (chart) wants a different default than just
{font}
(wants to define the default font to something else than whatfont
function defines) there are two options:- Chart don't want the theme information to override this:
Set default to
{font family='arial'}
- Chart would still want the theme info to override this if its present
Set the default to
{font family={theme 'font.family' default='arial'}}
Proposed
theme
function arguments:Benefits:
theme
function has access totheming service
.If we were to implement different theming service in canvas vs dashboard, canvas would only need to override
theme
function, everything else would stay the same (not that we should actually do that, we should go with one theming implementation imo)Allows us to implement themes for any visual property (actually for every argument), allows us to introduce global overrides at a later point as well as user overrides.
No need to migrate anything. Canvas functions can stay exactly the same. Canvas stored expressions can stay exactly the same, lens expressions can stay exactly the same. (we'll take a look at this under examples)
Support for embeddables comes almost for free (we'll take a look at this under variables)
Examples:
Lets take a look at some expressions, and how would they look after interpreter parses them and inserts default arguments.
A simple
pie
expression with all the arguments defaultedpie
pie font={font} palette={palette}
interpreter inserted default argumentspie font={font family={theme 'font.family'} size={theme 'font.size'}} palette={palette name={theme 'palette.name'}}}
interpreter inserted default arguments for font and palette functions as well
we can see that a stored expression with pie function where styling arguments were defaulted (not set) will suddenly start supporting the themes
A
pie
expression with some arguments set (not-defaulted)pie font={font size=26}
pie font={font size=26 family={theme 'font.family'}} palette={palette name={theme 'palette.name'}}
interpreter sets the defaults for the missing arguments (defaulted), for which theming will work,
for arguments that were overriden the theming won't override them (expected behaviour)
Setting a different default on a chart function then the default of styling function is
pie font={font family={theme 'font.family' default='arial'}}
if theme 'font.family' is not set use arial defaultImplemented with variables:
pie font={font family={theme default='arial' } size={theme 'font.size'}}
pie font={font family={var 'theme.font.family' default='arial' } size={var 'theme.font.size'}
Everything needed to implement this is already in place, steps we need to take to make this a reality:
- agree on
theming
variables we'll use- change default values of styling functions to use the agreed on variables:
- For example:
font
function default forsize
is 14. we need to change it tovar 'theme.font.size' default=14
which will give us same result if theme info is not present,-> but if present it will use theme information
- Give variables to executor (and build UI around them)
- Add variables to embeddable base input (to make it work with embeddables directly (inside and outside of canvas))
Concerns:
- We loose type safety (as we anyway do with variables. Variable can be of any type so var function can return anything and we can't check that at compile time but only at runtime)
- Could we have default/forced theme variable for specific arguments ? To avoid using wrong variables for wrong arguments
- We should have theme variables as a separate high level concept to prevent colisions
Proposal 2:
Similar to proposal 1, but to solve type safety concert and concert with having wrong variables specified: instead of having a special
theme
function each of the styling functions should work directly with thetheming service
. Styling functions should have no default arguments defined on the expression function definition, but rather have the defaults asigned inside the function when the argument is undefined.Benefits:
Chart functions don't need to change but styling functions do. However each styling function needs access to the theming service.
Implementing of user overrides or global overrides might be more complicated as we'll need to do that in several styling functions (vs single function in proposal 1), but we might make that easier providing a set of utility functions each styling function would then use. (like
getThemeProperty('font.family', 'arial')
and that function internally can check the theme, global overrides, user overrides)We still don't need to migrate anything. No arguments to any functions would be changed, just styling function implementations would know that when there is no argument provided they should check with theming service for availability of it.
Support for embeddables still comes almost for free if we would be using variables for passing down theme information. Another option would be a global theming service (which imo should be avoided).
We avoid 1st and 2nd concert of proposal 1 with the downside of multiple functions needing access to theming service and implementations of those styling functions will be more complicated.
Examples:
pie
pie font={font}
// interpreter inserts default argument, however font doesn't have defaults set for any of its arguments// Internally font uses a getThemeProperty function when arguments were not given and provide default for when theme is not available
TLDR:
No matter which of above proposals we would go with we first need to agree on context types for our visual attributes (
font
,palette
, ....), provide base set of functions to work with those context types and have all charting functions exposing arguments allowing to set this. Once that is in place we should be able to implement theming without breaking backward compatibility or needing migrations.The text was updated successfully, but these errors were encountered: