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

HTMLHelper default css class names #17412

Open
Tracked by #8573
bigcitydev opened this issue Nov 26, 2019 · 24 comments
Open
Tracked by #8573

HTMLHelper default css class names #17412

bigcitydev opened this issue Nov 26, 2019 · 24 comments
Labels
affected-medium This issue impacts approximately half of our customers api-suggestion Early API idea and discussion, it is NOT ready for implementation area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates enhancement This issue represents an ask for new feature or an enhancement to an existing one feature-mvc-razor-views Features related to the Razor view engine for Razor pages and MVC views Priority:1 Work that is critical for the release, but we could probably ship without severity-major This label is used by an internal tool
Milestone

Comments

@bigcitydev
Copy link

bigcitydev commented Nov 26, 2019

I want to change default css class names to support bootstrap 4.

#4819

Please look on the bottom of the issue for context and explanation

I could not believe the values were hardcoded, but after 3.0 upgrade they remove the possibility to even change them.

Is there any workaround without using tag helpers ?

(Please dont tell me to use css rules for the default classes)

@javiercn javiercn added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Nov 26, 2019
@alexandrejobin
Copy link

With Core 2.2, we were able to override the default styling of the error messages by doing the following in the Startup.cs file:

public class Startup
{
    private readonly IConfiguration configuration;
    private readonly IHostingEnvironment hostingEnvironment;

    public Startup(IConfiguration configuration, IHostingEnvironment hostingEnvironment)
    {
        //logger.LogInformation("Application starting.");

        this.configuration = configuration;
        this.hostingEnvironment = hostingEnvironment;

        ChangeDefaultValidationClassesForBootstrap4ValidationClasses();
    }

    // This method gets called by the runtime. Use this method to add services to the container.
    public void ConfigureServices(IServiceCollection services)
    {
        services.Configure<CookiePolicyOptions>(options =>
        {
            // This lambda determines whether user consent for non-essential cookies is needed for a given request.
            options.CheckConsentNeeded = context => true;
            options.MinimumSameSitePolicy = SameSiteMode.None;
        });

        services
            .AddMvc()
            .SetCompatibilityVersion(CompatibilityVersion.Version_2_2);
    }

    // This method gets called by the runtime. Use this method to configure the HTTP request pipeline.
    public void Configure(IApplicationBuilder app)
    {
        if (hostingEnvironment.IsDevelopment())
        {
            app.UseDeveloperExceptionPage();
        }
        else
        {
            app.UseExceptionHandler("/Home/Error");
            app.UseHsts();
        }

        app.UseStaticFiles();
        app.UseCookiePolicy();
        app.UseMvc();
    }

    /// <summary>
    /// Change from default validation CSS classes to 'Bootstrap 4' validation classes.
    /// In particular it changes values of a bunch of static readonly variables in HtmlHelper class.
    /// </summary>
    private void ChangeDefaultValidationClassesForBootstrap4ValidationClasses()
    {
        SetPublicFieldValue<HtmlHelper>(nameof(HtmlHelper.ValidationInputCssClassName), "is-invalid");
        SetPublicFieldValue<HtmlHelper>(nameof(HtmlHelper.ValidationInputValidCssClassName), "is-valid");
        SetPublicFieldValue<HtmlHelper>(nameof(HtmlHelper.ValidationMessageCssClassName), "invalid-feedback");
        SetPublicFieldValue<HtmlHelper>(nameof(HtmlHelper.ValidationMessageValidCssClassName), "valid-feedback");
        SetPublicFieldValue<HtmlHelper>(nameof(HtmlHelper.ValidationSummaryCssClassName), "validation-summary-errors alert alert-danger");
        SetPublicFieldValue<HtmlHelper>(nameof(HtmlHelper.ValidationSummaryValidCssClassName), "validation-summary-valid alert alert-success");
    }

    private static void SetPublicFieldValue<T>(string fieldName, object fieldValue)
    {
        FieldInfo fieldInfo = typeof(T).GetField(fieldName, BindingFlags.Public | BindingFlags.Static);

        if (fieldInfo == null)
        {
            throw new NullReferenceException(string.Format("Static field '{0}' not found in '{1}' class", fieldName, nameof(T)));
        }

        fieldInfo.SetValue(null, fieldValue);
    }
}

I don't remember where I took this sample code but it worked flawlessly until I upgrade my project to Core 3.0 where I get the error message:

System.FieldAccessException: 'Cannot set initonly static field 'ValidationInputValidCssClassName' after type 'Microsoft.AspNetCore.Mvc.ViewFeatures.HtmlHelper' is initialized.'

We should be able to set the css class dynamically so we can support what ever css framework that we want.

@mkArtakMSFT mkArtakMSFT added the enhancement This issue represents an ask for new feature or an enhancement to an existing one label Nov 26, 2019
@mkArtakMSFT mkArtakMSFT added this to the 5.0.0-preview1 milestone Nov 26, 2019
@mkArtakMSFT
Copy link
Member

Thanks for contacting us.
This is indeed not possible at the moment. We will look into this later during 5.0 timeframe.

@bigcitydev
Copy link
Author

bigcitydev commented Nov 26, 2019

@mkArtakMSFT

Why is not possible for example to remove the readonly from the css class strings?

So we can use 3.1 release with the workaround.

Net Core is all about cross platform and then you can't even switch CSS frameworks if you are not a rocket scientist.

For me this is a fat bug :-)

In my opinion not acceptable for Long Term Support Release.

@ctolkien
Copy link
Contributor

Seeing as the other thread was closed due to "not enough community involvement", allow me to add a useless comment to signify that this is a big and consistent pain point with using MVC.

@pellesantiago
Copy link

Indeed, i will like to set my own css classes for validation.
Please fix it. Remove the readonly and the workaround will keep working as net 2.2.
Thank you

@FWest98
Copy link

FWest98 commented Mar 29, 2020

I would really like this feature as well. I found a temporary work-around, by making a custom HtmlGenerator and let the DI system inject that version in the html builders, instead of the DefaultHtmlGenerator. This way, you can replace the "incorrect" classes by your own custom ones.
For an example, I created a gist: https://gist.github.com/FWest98/9141b3c2f260bee0e46058897d2017d2 I also included a piece of code to update the jQuery ubobtrusive validation library as well.

@mkArtakMSFT
Copy link
Member

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

@REscobar
Copy link

REscobar commented Jul 9, 2020

I agree with the request, remembering to add a taghelper to every single input is quite painful.

I am using Bootstrap 4 and would like to set the validation class to something else other than the current.

Is there any workarround to not add an aditional tag to every single input, like create an Input tag helper that would check the model state and add the correct class?

@FWest98
Copy link

FWest98 commented Jul 9, 2020

The workaround I linked above (https://gist.github.com/FWest98/9141b3c2f260bee0e46058897d2017d2) replaces the classes that are added, such that you can modify the exact classes you want for validation success and fail. The gist contains fixes for both the frontend jQ validation code, as well as the backend model state code.

@REscobar
Copy link

@FWest98 Somehow I missed your previous post with the workaround, your workarround fits like a glove, thanks

@javiercn javiercn added affected-medium This issue impacts approximately half of our customers severity-major This label is used by an internal tool labels Oct 9, 2020 — with ASP.NET Core Issue Ranking
@lukesampson
Copy link

I think I'm blocked from upgrading to .NET 5.0 because of this. There is a workaround in .NET Core 3 where you could set the ValidationInputCssClassName property through reflection (I know, I know this is a "bad way to do it", but it worked). It seems this is no longer allowed in .NET 5.0.

I would prefer that the API didn't prevent customising the CSS class names to accommodate a web framework like Bootstrap, or just for a handmade website where you want a fine degree of control over the HTML that is output. It seems like such a necessary thing to provide in this API.

Can you please (re-)consider changing these properties from readonly static to static? Even if it's static ValidationInputCssClassName { get; private set; } that would (I think) allow setting the property through reflection if you insist that people shouldn't normally be able to override the defaults (I would strongly disagree, but whatever).

I'd prefer not to litter my code with a bunch of workaround code to hack it using the DI system as suggested above (props for figuring that out and sharing, but no thanks).

@pellesantiago
Copy link

I agree. Framework have to be closed but extensible.

I think I'm blocked from upgrading to .NET 5.0 because of this. There is a workaround in .NET Core 3 where you could set the ValidationInputCssClassName property through reflection (I know, I know this is a "bad way to do it", but it worked). It seems this is no longer allowed in .NET 5.0.

I would prefer that the API didn't prevent customising the CSS class names to accommodate a web framework like Bootstrap, or just for a handmade website where you want a fine degree of control over the HTML that is output. It seems like such a necessary thing to provide in this API.

Can you please (re-)consider changing these properties from readonly static to static? Even if it's static ValidationInputCssClassName { get; private set; } that would (I think) allow setting the property through reflection if you insist that people shouldn't normally be able to override the defaults (I would strongly disagree, but whatever).

I'd prefer not to litter my code with a bunch of workaround code to hack it using the DI system as suggested above (props for figuring that out and sharing, but no thanks).

@FWest98
Copy link

FWest98 commented Mar 2, 2021

I would be happy to contribute to an implementation of this issue, but what direction should the solution take? Should it be some DI-able Config object? Any guidance from the .NET team on this?

@pellesantiago
Copy link

I'm not from the NET team, but i suggest that all other functions that get access to that static variables be changed to access an overridable function that read that static variable. So in the future anybody can override the function and change the behavior without impact.

I would be happy to contribute to an implementation of this issue, but what direction should the solution take? Should it be some DI-able Config object? Any guidance from the .NET team on this?

@Gartenschlaeger
Copy link

Gartenschlaeger commented Apr 4, 2021

Any update on that? This i realy an annoying behavior currently. It cannot be that hard to have it configurable.
For such a great framework its a bit frustrating.

@javiercn javiercn added the feature-mvc-razor-views Features related to the Razor view engine for Razor pages and MVC views label Apr 18, 2021
@FWest98
Copy link

FWest98 commented Jul 27, 2021

@KaiGartenschlaeger I made a first implementation for this issue (PR #34777). Let's hope this works out

@FWest98
Copy link

FWest98 commented Aug 1, 2021

Adding API proposal as requested by @pranavkm (#34777 (comment))

Proposed API

Proposed API changes:

namespace Microsoft.AspNetCore.Mvc.ViewFeatures
{
    public class HtmlHelperOptions
    {
+       public string ValidationInputValidCssClassName { get; set; }
+       public string ValidationInputInvalidCssClassName { get; set; }
+       public string ValidationFieldValidCssClassName { get; set; }
+       public string ValidationFieldInvalidCssClassName { get; set; }
+       public string ValidationSummaryValidCssClassName { get; set; }
+       public string ValidationSummaryInvalidCssClassName { get; set; }
    }

    public class HtmlHelper
    {
-       public static readonly string ValidationInputCssClassName;
-       public static readonly string ValidationInputValidCssClassName;
-       public static readonly string ValidationMessageCssClassName;
-       public static readonly string ValidationMessageValidCssClassName;
-       public static readonly string ValidSummaryCssClassName;
-       public static readonly string ValidSummaryValidCssClassName;
    }
}

Usage Examples

Setting custom CSS class names, for example through DI:

// In Startup::ConfigureServices
services
    .AddMvc()
    .AddViewOptions(options =>
    {
        options.HtmlHelperOptions.ValidationInputValidCssClassName = "custom";
    });

This aligns with the current options already available in HtmlHelperOptions, for setting the element name/type for these validation items (ValidationMessageElement and ValidationSummaryMessageElement).

Alternative Designs

There are three options regarding the old readonly strings in HtmlHelper:

  • Remove them now: clearest way forward, only one place in the code base where these values are defined and used. However, it might break customer code using these values.
  • Don't remove them: best code compatibility, but it will be confusing why these values are there if they are not used anymore. It might be confusing for users where to look for customising these class names.
  • Mark them obsolete, remove them later. In my opinion the best way forward; it achieves the clarity of a single place to obtain the customization, while maintaining code compatibility for some time.

Risks

Removing altogether would be a breaking change, going for any of the two other options would not have major risks to my knowledge.

@pranavkm pranavkm added the api-ready-for-review API is ready for formal API review - https://github.com/dotnet/apireviews label Aug 1, 2021
@pranavkm pranavkm removed the api-ready-for-review API is ready for formal API review - https://github.com/dotnet/apireviews label Aug 9, 2021
@pranavkm pranavkm added this to the .NET 7 Planning milestone Oct 19, 2021
@pranavkm pranavkm added the Priority:1 Work that is critical for the release, but we could probably ship without label Oct 28, 2021
@topfunet
Copy link

Any update?

@FWest98
Copy link

FWest98 commented Jan 28, 2022

Unfortunately, the PR has been closed (#34777 (comment)) as this issue will be addressed in a bigger scope (#8573)

@davemasters1984
Copy link

The workaround I linked above (https://gist.github.com/FWest98/9141b3c2f260bee0e46058897d2017d2) replaces the classes that are added, such that you can modify the exact classes you want for validation success and fail. The gist contains fixes for both the frontend jQ validation code, as well as the backend model state code.

Thank you so much for this - works great.

I'm really surprised that something so obviously in need to being easily configurable/extensible requires this level of code change (which is not documented or discoverable). I'm also surprised that more people aren't complaining about this issue

@Kumima
Copy link

Kumima commented Jul 24, 2022

This problem is well solved in Blazor, but still not solved for this. But when using Identity in Blazor, we have to use Ragzor Page or MVC, which really sucks. Good features are separated.

@ghost
Copy link

ghost commented Sep 19, 2022

Thanks for contacting us.

We're moving this issue to the .NET 8 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s).
If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues.
To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

@FWest98
Copy link

FWest98 commented Sep 19, 2022

Will anything like this ever be addressed? Both this and #8573 have now been moved another iteration - how come we are still stuck with decade-old validation technologies?

@lukesampson
Copy link

I would still love to see this customisable in .NET. Recently came up against this issue again in a project transitioning from .NET Core 3 and had to write a CustomHtmlGenerator to manually search and replace classes in the class name attribute after .NET adds the wrong class name. It's strange that we have to jump through this particular hoop, and jarring to find that we don't have good control over such an essential and simple part of our HTML output.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affected-medium This issue impacts approximately half of our customers api-suggestion Early API idea and discussion, it is NOT ready for implementation area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates enhancement This issue represents an ask for new feature or an enhancement to an existing one feature-mvc-razor-views Features related to the Razor view engine for Razor pages and MVC views Priority:1 Work that is critical for the release, but we could probably ship without severity-major This label is used by an internal tool
Projects
None yet
Development

Successfully merging a pull request may close this issue.