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

Add border width utility #31484

Merged
merged 11 commits into from
Sep 9, 2020
Merged

Add border width utility #31484

merged 11 commits into from
Sep 9, 2020

Conversation

MauricioHernanCabrera
Copy link
Contributor

@MauricioHernanCabrera MauricioHernanCabrera commented Aug 16, 2020

I added border-width utility. it was a request #31299

Example

<div class="border border-primary border-width-5">Hello world</div>

<div class="border border-primary border-width-2">Hello world</div>

Fixes #31299

Demo: https://deploy-preview-31484--twbs-bootstrap.netlify.app/docs/5.0/utilities/borders/#border-width

@MauricioHernanCabrera MauricioHernanCabrera requested a review from a team as a code owner August 16, 2020 19:48
@MauricioHernanCabrera
Copy link
Contributor Author

I have added the border-width utility and a variable for handle the sizes

@MartijnCuppens
Copy link
Member

I think we can leave the width out here (eg. border-2). Also, this needs documentation.

@MauricioHernanCabrera
Copy link
Contributor Author

I think we can leave the width out here (eg. border-2). Also, this needs documentation.

Okay, What do you mean by documentation?

@MartijnCuppens
Copy link
Member

Okay, What do you mean by documentation?

Explain how these utilities need to be used on our documentation site. You'll probably need to edit this file:

https://github.com/twbs/bootstrap/blob/main/site/content/docs/5.0/utilities/borders.md

@MauricioHernanCabrera
Copy link
Contributor Author

Okay, What do you mean by documentation?

Explain how these utilities need to be used on our documentation site. You'll probably need to edit this file:

https://github.com/twbs/bootstrap/blob/main/site/content/docs/5.0/utilities/borders.md

Thanksss. I am going to work on the docs

@MauricioHernanCabrera
Copy link
Contributor Author

wooow the documentation in bootstrap is beatiful. I am learning a lot of front with this project

@MauricioHernanCabrera
Copy link
Contributor Author

@MartijnCuppens can I do some more por bootstrap? I want to contribute more to this project

@calvinkarpenko
Copy link

Would it not be better to have these in rem units rather than px units?

@MartijnCuppens
Copy link
Member

Would it not be better to have these in rem units rather than px units?

We don't do that for borders. Browsers will always render them in px, so if the default browser font size is changed, the borders could be blurred because they are for example 1.25px.

@MauricioHernanCabrera, could you rebase this branch, it contains lots of unrelated changes?

@MauricioHernanCabrera
Copy link
Contributor Author

Would it not be better to have these in rem units rather than px units?

We don't do that for borders. Browsers will always render them in px, so if the default browser font size is changed, the borders could be blurred because they are for example 1.25px.

@MauricioHernanCabrera, could you rebase this branch, it contains lots of unrelated changes?

done!

MartijnCuppens
MartijnCuppens previously approved these changes Aug 27, 2020
Copy link
Member

@MartijnCuppens MartijnCuppens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. ping @twbs/css-review for second opinion

@XhmikosR
Copy link
Member

XhmikosR commented Sep 1, 2020

@mdo WDYT?

@XhmikosR XhmikosR requested a review from mdo September 1, 2020 15:56
@mdo
Copy link
Member

mdo commented Sep 1, 2020

Can we get a mention on the migration.md page? Otherwise LGTM!

@XhmikosR
Copy link
Member

XhmikosR commented Sep 8, 2020

I'm not sure the example is right, we should probably add margin right in the examples?

@MauricioHernanCabrera
Copy link
Contributor Author

I'm not sure the example is right, we should probably add margin right in the examples?

okasss, I'm going to work on that

@MartijnCuppens MartijnCuppens dismissed their stale review September 9, 2020 08:22

Example class is missing

@XhmikosR XhmikosR changed the title Border width utility Add border width utility Sep 9, 2020
@XhmikosR XhmikosR merged commit 6455c2e into twbs:main Sep 9, 2020
olsza pushed a commit to olsza/bootstrap that referenced this pull request Oct 3, 2020
Co-authored-by: XhmikosR <[email protected]>
Co-authored-by: Martijn Cuppens <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Provide way to specify size to border
5 participants