-
-
Notifications
You must be signed in to change notification settings - Fork 79k
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
Cannot customize button border color #23450
Comments
Although the border is defined at the mixin: It's set as the same color of the background here: I think this is a bug. The solutions I can think of are:
What do you think? |
I described the same problem here #23126 |
@andresgalante In my opinion this is one of the weakness with the new variable system. It is structured a lot better than earlier, but it isn't as flexible as it was, limiting us to either accept the default variations of basic color settings or to be forced to create our own definitions of the classes which I think will create more confusion, larger style sheets, more error prone and harder to read. As for your suggestions, I think that the first one isn't an option. In our case, we want the background to be one color and the outer to be a completely other one ($white background and $blue border) and as far as I understand your suggestion it won't give me that. The only way this can be solved for the usage I have is to be able to configure colors by themselves, and the only way is to have variables. However, instead of having lots of key/value pairs, why not use key/array structures instead? Like this: (I do not know scss that well, so just look at this example as pseudo code)
Then generate buttons based on this with:
and let As I said, it may not work since I don't know scss that well, but if it works it's worth a try. I did get the issue described by @wojtask9 today and was planning to check it out tomorrow but it is clearly already registered. But I think that a similar construct like this (if it works) could be applied to lots of the similar issues. |
@dozer75 you are absolutely right it is limiting and my solution wouldn't help much. |
@andresgalante Actually I think that this abstraction will make it easier to read because users of the scss wont need to go into the source to find out why for example a button get's a specific border or foreground color or an alert get's it's border color or foreground color as we have to do now. By having it structured in the variables files with logical variable names it can be seen there. I have used two full days now with the current scss files to try to understand what's happening under the hood and it is quite confusing with all these auto calculated variants inline in functions. On the plus side it has given (or forced :) ) me the opportunity to learn scss more in-depth than I ever had before. |
You are right @dozer75 I think it's worth the try, but it's not really my call 😄 If you do it I would love to review that PR and learn from it. |
I found this because after updating from Alpha 6 to Beta I noticed that my own Button variations looked completely messed up. It appears as though I can't directly control the text colour because of Even having $brand-primary removed seemed like something that should have happened well before alpha 6. I feel like the mixins still need a lot more inline documentation. |
color-yiq is define in _functions.scss.
it works by contrasting the RGB values. If the output is >=150, the contrast is too high and the text colour is set to 111. It's a nightmare trying to override mixins in Bootstrap at the moment! #23499 |
@andresgalante My problem is that I don't have that much time, but I'll give it a try to structure something however not sure when I can get it done. There is a lot of parameters that would have to be restructured around. But maybe start small to get some idea. |
@dozer75 before you embark in something like this, I'd wait for the blessing from one of the core team, specially if you don't have much time. |
@andresgalante Ok, but I am a bit curious myself, so I will try to do a small prototype portion to see if it even is possible in a neat clean way :) It will also be a good experience anyway to be able to handle bootstrap in the future by learning more of the internals. |
@andresgalante I pushed a branch to a bootstrap fork on my account: https://github.com/dozer75/bootstrap/tree/enhance-button-variables Don't want to PR it as it is just to show one (of many) solutions to the issue. I can't find really a good way to do it, one is to push lots of variables into _variables.scss, but I found that it messed it up quite a lot. So I went for this one where the theme button maps was filled with default values if they doesn't exist. Most of the changes are in mixins/_buttons.scss, but also in the ordinary _buttons.scss (just calling to the mixins in _buttons.scss) and _variables.scss (comments on how to customize). |
I just ran into this as well. The variable changes are really bad, I was looking forward to v4 and now I'm not. |
Closing as dupe of #23418. |
@joshbuckley I agree. Customising the border colour is a seperate issue to #23418. |
Any progress here? |
In the current _buttons.scss file, the same color is sent in as background and border color. This was not the case earlier where we could specify in our customization specific border and background colors.
We need to be able to customize border colors separate from the background. Otherwise why have borders at all?
I find this related to #23418
The text was updated successfully, but these errors were encountered: