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

Blazor Dashboard does not actually build Telerik Themes #260

Closed
Jejuni opened this issue Sep 5, 2023 · 2 comments
Closed

Blazor Dashboard does not actually build Telerik Themes #260

Jejuni opened this issue Sep 5, 2023 · 2 comments

Comments

@Jejuni
Copy link

Jejuni commented Sep 5, 2023

Bug report

Both the documentation as well as the readme claim that the "Blazor Dashboard App" builds the Telerik Themes and uses variables to customize the themes, which it does not.

Relevant quote from the documentation (under "Bootstrap Notes"):

The Telerik Bootstrap theme is not the same as the Bootstrap framework (or styles), it is our own theme that uses the Bootstrap metrics and design approaches to fit into a Bootstrap layout better. It can also use customized variables from Bootstrap, and you can see one way to do that through building the SASS files for both Bootstrap and Telerik in the following sample app: Blazor Dashboard.

Relevant quote from the ReadMe:

  • customized SASS variables for Bootstrap
  • the Telerik Bootstrap SASS theme to also utilize those variables

However, looking over the architecture of the solution we can see that the _Host.cshtml simply includes a link to the Telerik Theme hosted via cdn: <link id="theme" href="https://blazor.cdn.telerik.com/blazor/4.5.0/kendo-theme-bootstrap/all.css" rel="stylesheet" />.

It DOES include the bootstrap.scss in the actual theme building and shows how to customize variables for bootstrap. Since the Telerik theme is not imported during the theme building process, however, those variables will not be used for the Telerik Theme.
I assume this should work the same as with the Angular Bootstrap Theme.

(The Angular documentation on the issue of sass customization of the themes is also much clearer and easier to follow than its Blazor equivalent.)

Expected Behavior

The "Blazor Dashboard" sample should show how to include the Telerik Theme in such a way that sass variables customization works, in the way both the documentation and the readme claim it does.

Note

The "Blazing Coffee" sample suffers from a similar problem. While the documentation doesn't claim to show off sass variables customization, it DOES include both @progress/kendo-theme-default as well as bootstrap in its package.json, but then doesn't do anything with it.

@dimodi
Copy link
Contributor

dimodi commented Sep 19, 2023

Hello @Jejuni ,

Indeed, our sample apps no longer build the themes from source, because this made them too complex for 99% of our customers who didn't need this.

If you like, you can use an older version of the blazor-ui repo, before this commit: a604241

Fixed with

@dimodi dimodi closed this as completed Sep 19, 2023
@Jejuni
Copy link
Author

Jejuni commented Sep 19, 2023

Hey @dimodi

If that is the direction Telerik wants to take that's fine.

I think it's quite a useful feature to be able to build the themes yourself and especially customize both bootstrap and kendo themes using the same variables.
I can't argue with the 99% figure as I don't have the data, but I find it hard to believe that most web developers would find a simple sass compilation workflow too complex to understand, especially as it isn't required to understand the rest of the sample.

The sample also still has all the code that increases complexity (package.json, gulp, sass, etc.). The only things changed basically boil down to removing @import "~@progress/kendo-theme-bootstrap/dist/all.scss"; in styles.scss and adding a cdn theme in your _Host.cshtml.

You guys also seem to have designed your bootstrap theme with this exact use-case in mind and I think it's really well done.
The Angular docs also describe how to do it, so I don't see why it needed to be removed from the blazor docs.
Updating the docs to correctly state how this is supposed to be done in would have been better in my opinion.

Thank you for providing a link to a working commit, though.

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

No branches or pull requests

2 participants