userfrosting / UserFrosting

Modern PHP user login and management framework
https://www.userfrosting.com
Other
1.64k stars 366 forks source link

Select2 css width override #1032

Open phillmac opened 5 years ago

phillmac commented 5 years ago

https://github.com/userfrosting/UserFrosting/blob/a8e6711fd5afd0572fc1581950f161b9581a8ffd/app/sprinkles/core/assets/userfrosting/css/userfrosting.css#L153

The !important on this line makes it incredibly hard to dynamically set a smaller width. This CSS file has the most priority over the other standard included CSS files, so the !important is effectively redundant.

E.g. $(".select2-container").css("width", "8%") doesn't work because the !important overrides the inline style. It's almost impossible to add a !important with jQuery, see: https://bugs.jquery.com/ticket/11173

To get around this problem, one has to resort to this sort of ugliness: $(".select2-container")[0].style.setProperty("width", "8%", "important")

phillmac commented 5 years ago

Just noticed that it's there to override the default inline style added by select2. Still, it's generally considered a code smell to have to use !important, especially like this in a framework because of this kind of situation

ritesh-pandey commented 5 years ago

Hi @phillmac! I want to contribute to this project.

I tried to understand the problem statement.

If I am correct, the default inline style added by select2 is width: 100% which is same as what we have in our stylesheet. So what are we overriding? Am I missing something?

Screenshot 2019-10-13 at 6 16 09 PM
phillmac commented 5 years ago

In some cases 100% width is not desireable. I found it really hard to force a smaller width for e.g. with inline text.

phillmac commented 5 years ago

If you look at the bottom of your screenshot there is a width 100% !important rule. My problem is with the presence of the !important making it hard to change.

ritesh-pandey commented 5 years ago

I agree to your point that we shouldn't have !important in our stylesheet. It is a bad practice.

But can you please give me an example where we have set width anything other than 100%? In current example (screenshot), the width in our stylesheet and default in-line width of select2-container--default is same i.e. 100%. So I am unable to understand the change in layout.

Also, select2 provides options to configure container width

phillmac commented 5 years ago

Thats exactly my point, the link you mentioned for the config option does not work because the !important takes precidence. I wanted to use an 8% width to fit the element inline with some text. I'll try and make some screenshots & example html

amosfolz commented 5 years ago

It looks like that line was changed 3 years ago. The problem it was intended to fix may not be an issue anymore. Have you tried removing it to see what happens?