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

Remove CSS settings from app-settings #17235

Open
ns8482e opened this issue Dec 13, 2024 · 7 comments
Open

Remove CSS settings from app-settings #17235

ns8482e opened this issue Dec 13, 2024 · 7 comments
Milestone

Comments

@ns8482e
Copy link
Contributor

ns8482e commented Dec 13, 2024

Today the CSS classes are defined in app-settings as following, this makes app-settings noisy - The purpose of app setting to be limited to application and feature configuration - that managed by configuration management and not for UX/UI management

   //"TheAdminTheme": {
    //  "StyleSettings": {
    //    "WrapperClasses": "mb-3",
    //    "LimitedWidthWrapperClasses": "row",
    //    "LimitedWidthClasses": "col-md-6 col-lg-4 col-xxl-3",
    //    "StartClasses": "",
    //    "EndClasses": "",
    //    "LabelClasses": "",
    //    "OffsetClasses": ""
    //   }
    //},

Like Zone, we don't provide Zones configuration in app-settings, instead we provide UI to manage zones
Similarly we should not have CSS settings in app settings -instead, theme should provide custom UI to manage such settings needed for the theme that is only applicable to that theme.

Possible options

  • Use HTML to define CSS classes
  • Use custom tag helper to inject custom classes instead of C# Helper methods
  • Provide Settings UI for the theme if specific theme provides configurable items for UI/UX users.
@MikeAlhayek
Copy link
Member

MikeAlhayek commented Dec 13, 2024

These options are used by the these helpers that are used everywhere

These configuration can be configured by any theme that is derived by TheAdmin theme to enable styling and small customization. Until we reusable components in TheAdmin theme that allows one to override the look and feel of TheAdmin theme, these should be be removed.

For example, I have an admin theme that places the labels on the same line as the input. I am able to do this now using these options. Here is an example

image

This is possible by adding the following

        services.PostConfigure<TheAdminThemeOptions>(options =>
        {
            options.WrapperClasses = "row mb-3";
            options.LimitedWidthWrapperClasses = "row";
            options.LimitedWidthClasses = "col-md-6 col-lg-5 col-xxl-4";
            options.StartClasses = "col-lg-2 col-xl-3";
            options.EndClasses = "col-lg-10 col-xl-9";
            options.LabelClasses = "col-form-label text-lg-end col-lg-2 col-xl-3";
            options.OffsetClasses = "offset-lg-2 offset-xl-3";
        });

What I suggest to do, is add theme settings UI to expose these options. The Themes UI is now shape base and you can add a settings button next to each theme where one can configure these values from the UI. In my project, I have this settings for each theme. currently, this allows me to inject css code in the theme. But you can use the same thing to expose these settings.

image

@hishamco
Copy link
Member

From a UI/UX perspective coding CSS classes is hard, it would be nice to let the HTML control everything. I still remember one of my friends 3 or 4 years ago when he told me, why the framework is tightly coupled on Bootstrap, this will limit the possibility of extending or changing the UI framework in the future

@MikeAlhayek
Copy link
Member

why the framework is tightly coupled on Bootstrap, this will limit the possibility of extending or changing the UI framework in the future

I agree. That's when having some sort of components design would be great. Until then, these options should stay to allow customizing our tightly couple theme.

@hishamco
Copy link
Member

I created a prototype long time ago, hopefully one day I will demo it

@sebastienros
Copy link
Member

this makes app-settings noisy

If this is the only issue, I believe there are ways to fix it, like @MikeAlhayek suggested by code. Or load other config files, splitting the config (is that possible?, something to add?)

@sebastienros sebastienros added this to the backlog milestone Dec 19, 2024
Copy link
Contributor

We triaged this issue and set the milestone according to the priority we think is appropriate (see the docs on how we triage and prioritize issues).

This indicates when the core team may start working on it. However, if you'd like to contribute, we'd warmly welcome you to do that anytime. See our guide on contributions here.

@ns8482e
Copy link
Contributor Author

ns8482e commented Dec 23, 2024

this makes app-settings noisy

If this is the only issue, I believe there are ways to fix it, like @MikeAlhayek suggested by code. Or load other config files, splitting the config (is that possible?, something to add?)

Current setting works for small org where few dev manaenge the app.

But in SME or LE, have separation of duties, some organizations have app support team they manage infrastructure and app configuration- but they are not admin or owner of the application. Designer who can control such css- do not have access to change such configuration on infra.

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

No branches or pull requests

4 participants