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 radius feature to core/image #1871

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

GP-Dan-Tovbein
Copy link
Contributor

@GP-Dan-Tovbein GP-Dan-Tovbein commented Dec 23, 2022

By Doing this we'll be able to potentially remove some custom classes for borders from here.

@mixin rounded-image-size($size) {
  figure,
  img {
    border-radius: 50%
    height: $size
    width: $size
  }

  img {
    object-fit: cover
  }
}

and

&.is-style-rounded-180 {
    @include rounded-image-size(180px)
  }

  &.is-style-rounded-90 {
    @include rounded-image-size(90px)
  }

What to do?

  1. Merge this feature
  2. Open a new PR removing all the appearances of those classes. For example from the Quick Links block pattern.

- Doing this, we'll be able to remove custom classes for borders like ".rounded-90"
@GP-Dan-Tovbein GP-Dan-Tovbein self-assigned this Dec 23, 2022
@GP-Dan-Tovbein GP-Dan-Tovbein requested review from comzeradd, mleray and a team December 23, 2022 16:36
@GP-Dan-Tovbein GP-Dan-Tovbein marked this pull request as ready for review December 23, 2022 16:36
planet-4 added a commit to greenpeace/planet4-test-mars that referenced this pull request Dec 26, 2022
/unhold c88a0720-f144-41cd-a315-55f2561360b0
@planet-4
Copy link
Contributor

planet-4 commented Dec 26, 2022

Test instance is ready 🚀

🌑 mars | admin | blocks report | CircleCI | composer-local.json

⌚ 2023.01.13 21:21:01

planet-4 added a commit to greenpeace/planet4-test-mars that referenced this pull request Dec 27, 2022
/unhold 1c267d5b-053d-483e-bc23-bfe4d1f9f265
planet-4 added a commit to greenpeace/planet4-test-mars that referenced this pull request Dec 27, 2022
/unhold 439bf3aa-7f05-4ad9-8477-73e8fb5b6f5a
planet-4 added a commit to greenpeace/planet4-test-mars that referenced this pull request Dec 28, 2022
/unhold 6cf21cb6-c0fe-4590-a8f4-a579064cdefa
Copy link
Contributor

@mleray mleray left a comment

Choose a reason for hiding this comment

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

This change will solve the border-radius issue, but the classes were also added to force the size of the images, I guess we would need to add a class for each size like the w-40 one you added for the Issues template?

@GP-Dan-Tovbein
Copy link
Contributor Author

This change will solve the border-radius issue, but the classes were also added to force the size of the images, I guess we would need to add a class for each size like the w-40 one you added for the Issues template?

Correct me if I see correctly your point! We wouldn't need to add custom class for border-radius since it's currently a default feature. I think the editor could now control that doesn't matter which image size he/she prefers. Am I right?

planet-4 added a commit to greenpeace/planet4-test-mars that referenced this pull request Jan 13, 2023
/unhold 9dde0dff-d8b7-448c-899f-607b59392dc4
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants