telerik / kendo-themes

Monorepo for SASS-based Kendo UI themes
148 stars 68 forks source link

Core module & styles reusability #4602

Open kspeyanski opened 1 year ago

kspeyanski commented 1 year ago

I will keep this issue to track progress towards sharing modules, styles and utilities between all available kendo-theme. This discussion was sparkled as part of our need to implement a common Color System across the themes which we can utilize to generate custom swatches, dark themes etc.

Another example for a shared module would be a motion module, a spacing module or virtually any constructive part of what we call a Design System.

Background

We have 6 themes:

Some of them reuse styles, and other reuse styling approaches, but they all differ in some way or other. This complicates any tasks which involve common functionality across the themes, as is the case with the Color System.

To provide some examples, the three main themes (default, bootstrap and material) are mostly reusing whatever they can and also utilizing some functionality from the core module. However, the fluent theme is build from ground up in order to explore the updated @use/@forward syntax of sass and embraces the future of the framework. Additionally, the core module was also forked as part of a core folder instead of reusing the original module due to technical challenges. The nouvelle theme on the other hand had also explored the opportunity to configure it through css variables instead of sass. While this brought an useful insight for the team by the time it was developed, the discrepancy between it and the other themes introduces other challenges and further complicates any adoption of shared codebase.

First-steps

The changes introduced with https://github.com/telerik/kendo-themes/pull/4518 were in order to find a way to introduce the concept of the core module across all themes, and attach it as a dependency mainly in the fluent theme, but also explore how the themes are configuring the core module.

However, with the above mentioned PR we've only linked all the themes and the core module, but we're yet to put reusable stylesheets there.

Failed attempts

We first tried to convert the current themes into a configuration-only modules, meaning that the core module would expose multiple APIs (though sass-variables) which would allow any theme to configure it according to its requirements without the need to write any theme-specific styles.

Building such complex configuration with multiple styling approaches behind toggle flags provided worst DX than anticipated. Main reason was the lack of type safety in the stylesheets and the global nature of sass-variables (when using the @import syntax). Variables' usage was not as straightforward and predictive as we hoped it would be, and debugging problems was a pain.

Another problem we faced was how different the requirements of each themes were. One specific example that stood out was how different themes required different color progression through it's states. While the bootstrap theme expects darker colors for a button's hover, focus and active states, the material theme required the opposite — a lighter one:

image

This is also as a result of different approaches used to style the components in their respective theme, but nevertheless showcase the need to have selective reusability across the themes. While themes like default and bootstrap prefers solid colors, the material design guideline suggest implementing a state layer which is applied on top of the original color.

Another scenario which stood out was how different the focus indicators are between the themes. One looks like a border, other is definitely a box-shadow and third is well... an :after element:

image

While multiple approaches can be toggle on and off by a variable (e.g. $kendo-use-solid-colors: true or $kendo-input-shadow), the lack of type safety and a module system (due to @import usage) makes things hard to follow, pain to debug and slow to develop.

Moving forward

We aim to use the core module to put any styles that are shared between all themes, but do this selectively while also allowing each separate theme to implement it's specific styling approaches while working with common variables dictated by the core theme.

A nice abstraction already in-place is the split between _layout and _theme styles for each component. While some themes are already re-exporting the default theme's styles, we believe a better place for all styles related is the core module, while the default theme is to also consume it and apply the theme-related styles according to it's design guideline. The core module should also be used as a starting point for any new theme.

The above mentioned reconstruction would, in theory, allow us to spin-off a style-less (headless!?) theme, which only provides the bare-minimum amount of styles needed for our components to function property, without any colouring and design aspects to it.

In order to achieve this, we should either:

The latter would be a BREAKING CHANGE for our themes, but comes with great benefits in terms of modularity, DX and performance.

A step further

Adopting the @use/@forward is a big part of this effort, and would also force some additional changes.

Currently, we're utilizing a big number of variables exposed for configuration. E.g. for each individual component we have a pair of $kendo-*-padding-x and $kendo-*-padding-y. Same goes for most variables (a notable exception here is the $kendo-spacing map) which.

With how explicit the @use ... with syntax is, instead of the current approach of setting each individual variable one by one:

$kendo-color-primary: #f35800;
$kendo-color-primary-lighter: #ffccaa;
$kendo-color-primary-darker: #03a9f4;
$kendo-color-secondary: #e9e9e9;

@import "@progress/kendo-theme-default";

We're looking into combining entire modules into maps. This will allow us to distribute them independent of each other. For example, this is how we imagine theme configuration to look like:

@use "sass:map";
@use "@progress/kendo-theme-default/sizing" as sizing;
@use "@progress/kendo-theme-default/swatches" as swatches;

$compact-spacing: map.deep-merge(sizing.$default, (
  sm: 1px,
  md: 2px,
  lg: 2px
));

@use "@progress/kendo-theme-default" with (
    $kendo-spacing: $compact-spacing,
    $kendo-colors: swatches.$indigo-dark
);

While this might look like removing granular customisation variables, we plan to actively be working towards reducing our css selector's specificity, which would enable developers to easily override smaller parts of our components. Additionally, we aim to greatly increase the predictability of where (and how) a specific variables is expected to be used to achieve the component's style.

So instead of:

$kendo-padding-x: 4px;

We would be encouraging selectors like:

.my-kendo-button:disabled {
    padding: 4px;
}

Once we've completely adopted the core module across all themes we can then fully work towards implementing common functionalities and modules like a shared Color System, a Motion module, or everything considered part of a our Design System.


As always, please let us know of any feedback you might have and we would be happy to sparkle a discussion!

iRoyPeleg commented 7 months ago

Hey @kspeyanski, any update on this? We're eagerly waiting for this feature to be implemented. The slowness described here (#4394 ) prevents us from moving forward with our projects.

Juveniel commented 7 months ago

Hey @iRoyPeleg,

We are monitoring the support for node-sass and dart-sass ( usage statistics ), and also the sass team announcement on when the support for @import will officially be deprecated ( see ref ) and will align the migration to @use and @forward accordingly.

In the meantime, you can use the embedded-sass to compile the themes from source. The embedded-host-node is part of the sass package since version 1.63.0, thus you can just pass the --embedded flag to switch to it with sass --embedded.

iRoyPeleg commented 7 months ago

Hey,

Unfortunately, it is not possible to use sass --embedded when using Angular-CLI (Won't Support Reason).

The official deprecation might take years. Quote from SASS Import Documentation:

The Sass team discourages the continued use of the @import rule. Sass will gradually phase it out over the next few years You could argue it's already deprecated according to this (very old) comment.

Angular Material moved to SASS Modules syntax more than 2 years ago. Other options are to use CSS Variables instead for theming.

Our only course of action is to not update kendo at all, as updating it will literally freeze our CIs.

Juveniel commented 7 months ago

@iRoyPeleg thanks for the additional details regarding the Angular CLI support.

Other alternative is to use the flattened all.scss file that is shipped in the theme distribution, which workarounds the compilation issue (all dependencies are flattened in the same file ), thus you should be able to quickly compile when importing the theme from this file:

// theme.scss
@import "@progress/kendo-theme-default/dist/all.scss";

Tested with the sass dep and results in roughly ~1 sec build time:

npx sass theme.scss theme.css --load-path=node_modules

The important bit is to use the file from the dist folder, and not the one from the scss folder when importing the theme.

iRoyPeleg commented 7 months ago

Hey,

I'll list the reasons why that solution doesnt work for us:

This is why we have no incentive to use all.scss file.

Juveniel commented 7 months ago

This makes things more clear. The use case is very specific and indeed the above two solutions will not work. You can possibly switch the Angular CLI to use the legacy API via NG_BUILD_LEGACY_SASS=1 as a temporary solution or wait until we migrate the themes to the new sass module system.

As for the migration itself - a lot of users are still using node-sass, thus it is important to make sure that the migration happens before the official deprecation of @import by the sass team, but also make sure that this is executed as gently as possible with minimal disruption. I will make sure to update the main issue as soon as I have additional infromation with specific timeframes for the migration.

iRoyPeleg commented 7 months ago

Thanks, looking forward to it.

nadavsinai-philips commented 7 months ago

I just checked npm stats for node-sass and sass (dart sass) - see here it seems node-sass has 10 times less traffic and keeps decreasing It has a big deprecation notice in the top of the NPM page and I don't think it should be of any consideration with regards to the '@import' migration issue in Kendo.

Currently building of custom theme kendo sass is too slow to be useable

iRoyPeleg commented 5 months ago

Hey @Juveniel, Hope everything is fine. Any update on the progress of this issue?