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

Adding background-color and border-width variables to HR element #33815

Closed
wants to merge 2 commits into from
Closed

Adding background-color and border-width variables to HR element #33815

wants to merge 2 commits into from

Conversation

devhoussam
Copy link
Contributor

Fixes #33793

@devhoussam devhoussam requested a review from a team as a code owner May 3, 2021 22:13
@devhoussam devhoussam changed the title Adding background-color and border-width to HR element Adding background-color and border-width variables to HR element May 3, 2021
@ffoodd
Copy link
Member

ffoodd commented May 4, 2021

Thanks for the PR :)

I don't see the point of this: currentColor makes inheriting color useful (so if one override this variable, $hr-color doesn't make sense) and resetting border is, well, a reset.

I don't see any use case for this, how would it be useful?

@devhoussam
Copy link
Contributor Author

Thanks for the PR :)

I don't see the point of this: currentColor makes inheriting color useful (so if one override this variable, $hr-color doesn't make sense) and resetting border is, well, a reset.

I don't see any use case for this, how would it be useful?

I think it is important because it exists and is already used and it would be better to edit it by using variables only.

@ffoodd
Copy link
Member

ffoodd commented May 4, 2021

Why editing it at all? What's the use case?

@GeoSot GeoSot added the css label Jun 16, 2021
scss/_reboot.scss Outdated Show resolved Hide resolved
scss/_variables.scss Outdated Show resolved Hide resolved
@mdo mdo added the v5 label Jul 26, 2021
@mdo
Copy link
Member

mdo commented Oct 5, 2021

Pushed to #35123.

@mdo mdo closed this Oct 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants