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

WIP: Add support for images in icon-grids #103

Closed
wants to merge 3 commits into from
Closed

WIP: Add support for images in icon-grids #103

wants to merge 3 commits into from

Conversation

pleasantone
Copy link
Contributor

@pleasantone pleasantone commented Jan 11, 2024

@razonyang
Copy link
Member

Thanks, but it doesn't look well (it's a bit of a waste of space on page), and the main reason about separating into new shortcode is that they are used in different purposes. I'd like to that each shortcode is only responsible for doing one job well.

@pleasantone
Copy link
Contributor Author

pleasantone commented Jan 12, 2024 via email

@razonyang
Copy link
Member

image

Screenshot captured from local.

@pleasantone
Copy link
Contributor Author

Ahh, I see, yours looks quite different than mine, but I was also mis-using description and caption instead of title...

when I changed to title and added me-2 (or the #float-start fragment (see wrapperClass issue in images, it looks much better...

Screenshot 2024-01-12 at 10 50 05 AM

@pleasantone pleasantone changed the title Add support for images in icon-grids WIP: Add support for images in icon-grids Jan 12, 2024
@razonyang
Copy link
Member

razonyang commented Jan 13, 2024

I've thought about this, but I don't want to complex the icon-grid shortcode, since:

  1. Hard to maintain: the icon-grid, there are some parameters for icon only, such as the alignment, when set as center, the icon will be align to center, but it also affects the image, it's hard to maintain different parameters (layouts) for both of icon and images.
  2. Confused: the icon-shortcode name is confused if we support image.

I've created another shortcode (#105) for #100.

Preview:

image

Let me know if the new shortcode suit your needs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants