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.

Sign up for GitHub

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

Jump to bottom

Close #2598 #2630

Merged
merged 3 commits into from Aug 4, 2020
Merged

Close #2598 #2630

merged 3 commits into from Aug 4, 2020

Conversation

service-paradis
Copy link
Contributor

This is a improvement. It adds a new customizable variables to change the card component border radius.

Proposed solution

  • Add $card-radius variable
  • Default to 0
  • Update variables/card.json

Tradeoffs

This is not a breaking change.

Testing Done

Yes, I made some tests using the local Bulma documentation.

Changelog updated?

No.

** This is my first Bulma PR. Tell me if there is anything wrong with it!

@Paul-Cl-ark
Copy link

Hey, this would be a great addition, and ideas if and when it'll be merged?

@service-paradis
Close jgthms#2598
7eafdd0 
- Add $card-radius variable
- Default to 0 (no braking change)
- Update variables/card.json
@jgthms
Copy link
Owner

jgthms commented Oct 12, 2019

Why not target .card directly? For example, if the :first-child is a .card-image, it won't affect it visibly because the image inside will cover the border-radius anyway.

I think it would be more effective to have the rule on .card with an additional overflow: hidden.

@service-paradis
Copy link
Contributor Author

service-paradis commented Oct 13, 2019 •
edited

Sure @jgthms , I can target .card directly.
It was somehow done like this for the message component. Could be easier to customize (if someone want only bottom radius for example). Could even be 2 variables eventually (card-radius-top and card-radius-bottom).

But I understand, the simplest way is probably better for now. Will submit a revision soon! Revision submitted!

@service-paradis
Fix per @jgthms
94cf677 
- Could be a small breaking change
   since `overflow: hidden` is added to card
@service-paradis
Merge branch 'master' into close-2598
1b8aeba
@MaskyS
Copy link

MaskyS commented Dec 16, 2019

bump @jgthms

@zegheim
Copy link

zegheim commented Aug 4, 2020

Any updates on this? Would be lovely to have! @jgthms

@jgthms jgthms merged commit aed4533 into jgthms:master Aug 4, 2020
@fabiofdsantos fabiofdsantos mentioned this pull request Nov 6, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
needs work
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@service-paradis @Paul-Cl-ark @jgthms @MaskyS @zegheim