Skip to content
This repository has been archived by the owner on Nov 18, 2024. It is now read-only.

Create CTA: grid with products and link pattern, alt #390

Merged
merged 10 commits into from
Sep 24, 2024

Conversation

jasmussen
Copy link
Contributor

@jasmussen jasmussen commented Sep 24, 2024

Description

Alternative to #273 and based on the work in #273 (comment). Props @graylaurenm for the work.

Screenshots

370261170-0727539f-debb-47d5-915e-2fdff263dc5b

Copy link

github-actions bot commented Sep 24, 2024

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: graylaurenm <[email protected]>
Co-authored-by: jasmussen <[email protected]>
Co-authored-by: juanfra <[email protected]>
Co-authored-by: carolinan <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

Copy link

github-actions bot commented Sep 24, 2024

Preview changes

You can preview these changes by following the link below:

I will update this comment with the latest preview links as you push more changes to this PR.
⚠️ Note: The preview sites are created using WordPress Playground. You can add content, edit settings, and test the themes as you would on a real site, but please note that changes are not saved between sessions.

@jasmussen
Copy link
Contributor Author

Note that for some reason this pattern isn't showing up for me in the pattern inserter. Not sure what I'm doing wrong.

Copy link
Member

@juanfra juanfra left a comment

Choose a reason for hiding this comment

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

Thanks for working on this, Joen!

I've tested this one and I see the pattern in the inserter. Although, the colored covers are looking smaller in that view and it doesn't represent the pattern perfectly. But that's likely unrelated to the PR. You can see what I referenced in the following screencast:

Screen.Recording.2024-09-24.at.14.20.06.mov

Regarding the pattern, everything looks good. The only thing I saw is that the grid items that only contain text are the only ones that are not looking squared on mobile.

The other unrelated Gutenberg issue I found, is that squared images are not working well on Firefox.

@jasmussen
Copy link
Contributor Author

Thanks for pushing the change, and for reviewing. I wonder if we can't fix the issue with collapsing text by making those transparent cover blocks with 1:1 aspect ratio too. I have to go to a very late lunch now, in case you get to it, feel free to push such a change. Otherwise I can take a look later.

The other unrelated Gutenberg issue I found, is that squared images are not working well on Firefox.

This is gutenberg core yes? Do you know if this is tracked already? Pretty sure that we're using web-standard aspect-ratio: 1/1; and object-fit: cover; to accomlish this, so hopefully it can be something to fix on the Firefox side, unless the block editor is doing something wrong.

@juanfra
Copy link
Member

juanfra commented Sep 24, 2024

I wonder if we can't fix the issue with collapsing text by making those transparent cover blocks with 1:1 aspect ratio too

Yes, that's what I was thinking. I've just pushed changes to wrap those texts in a cover block (1:1) rather than stack. It's working well for me now.

This is gutenberg core yes? Do you know if this is tracked already? Pretty sure that we're using web-standard aspect-ratio: 1/1; and object-fit: cover; to accomlish this, so hopefully it can be something to fix on the Firefox side, unless the block editor is doing something wrong.

Yes, it's Gutenberg core. I was checking this one out and it seems that Firefox needs to have the width and height on images for aspect-ratio/object-fit to work. I couldn't find any issue that's tracking this. .wp-block-image img already has a max-width of 100%; if we also add width to 100%, it should get resolved if we add a width of fit-content it should be resolved.

Screen.Recording.2024-09-24.at.15.18.42.mov

@carolinan
Copy link
Contributor

If you update the viewport width in the pattern file header to 1400 it will look like this in the inserter instead:
image

@jasmussen
Copy link
Contributor Author

Nice! Works well. Good to go? Thank you for the help, all.

@juanfra
Copy link
Member

juanfra commented Sep 24, 2024

That sounds good to me.

@juanfra juanfra merged commit 9b06cfd into trunk Sep 24, 2024
4 checks passed
@juanfra juanfra deleted the graylaurenm-cta-grid-products-link branch September 24, 2024 16:33
@jasmussen
Copy link
Contributor Author

🙏

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

Successfully merging this pull request may close these issues.

4 participants