This repository was archived by the owner on Feb 23, 2024. It is now read-only.
Add "Just Arrived Full Hero" commerce adjacent pattern #7812
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The release ZIP for this PR is accessible via: |
TypeScript Errors ReportFiles with errors: 428 🎉 🎉 This PR does not introduce new TS errors. |
|
Size Change: 0 B Total Size: 974 kB ℹ️ View Unchanged
|
nerrad
force-pushed
the
add/just-arrived-full-hero
branch
from
a993ba2 to
9f1fe75
Compare
|
LGTM - I assume that pink border on the button is coming from that theme style? |
Yup |
Aljullu
requested review from
a team and
dinhtungdu
and removed request for
a team
dinhtungdu
reviewed
Dec 5, 2022
patterns/just-arrived-full-hero.php
Outdated
| * Categories: WooCommerce | ||
| */ | ||
| ?> | ||
| <!-- wp:group {"align":"full","style":{"spacing":{"margin":{"top":"0px","bottom":"0px"}},"elements":{"link":{"color":{"text":"var:preset|color|black"}}}},"textColor":"black","layout":{"inherit":true,"type":"constrained"}} --> |
There was a problem hiding this comment.
What is the advantage of setting color for this block over using default color from theme? Beside, the textColor is set to a fixed color (black), which make this a bit harder to use on dark themes.
I think we should use the default color from theme when possible.
There was a problem hiding this comment.
Agreed. I'll remove the group and that will take care of that.
patterns/just-arrived-full-hero.php
Outdated
| ?> | ||
| <!-- wp:group {"align":"full","style":{"spacing":{"margin":{"top":"0px","bottom":"0px"}},"elements":{"link":{"color":{"text":"var:preset|color|black"}}}},"textColor":"black","layout":{"inherit":true,"type":"constrained"}} --> | ||
| <div class="wp-block-group alignfull has-black-color has-text-color has-link-color" style="margin-top:0px;margin-bottom:0px"><!-- wp:cover {"useFeaturedImage":true,"dimRatio":0,"minHeight":50,"minHeightUnit":"vw","contentPosition":"center center","isDark":false,"align":"full","style":{"spacing":{"padding":{"left":"50vw"}}}} --> | ||
| <div class="wp-block-cover alignfull is-light" style="padding-left:50vw;min-height:50vw"><span aria-hidden="true" class="wp-block-cover__background has-background-dim-0 has-background-dim"></span><div class="wp-block-cover__inner-container"><!-- wp:heading {"style":{"spacing":{"margin":{"top":"0px","right":"0px","bottom":"0px","left":"0px"}},"typography":{"fontSize":"40px","fontStyle":"normal","fontWeight":"700"}}} --> |
There was a problem hiding this comment.
Instead of a fixed value (40px), should we use a preset value here?
Aljullu
added
the
status: blocker
nerrad
force-pushed
the
add/just-arrived-full-hero
branch
from
9f1fe75 to
7c2fe48
Compare
|
@dinhtungdu I removed the group block which I think also takes care of the other issues you pointed out. I also tested on mobile and tablet previews and it still looks okay. |
Script Dependencies ReportThere is no changed script dependency between this branch and trunk. This comment was automatically generated by the |
|
LGTM 👌 |
dinhtungdu
reviewed
Dec 5, 2022
patterns/just-arrived-full-hero.php
Outdated
| <!-- /wp:heading --> | ||
|
|
||
| <!-- wp:paragraph {"style":{"typography":{"fontSize":"16px"}}} --> | ||
| <p style="font-size:16px">Our early autumn collection is here.</p> |
There was a problem hiding this comment.
This string should be wrapped in a __() call so it can be translated.
patterns/just-arrived-full-hero.php
Outdated
| <h2 id="just-arrived" style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px;font-size:40px;font-style:normal;font-weight:700">Just arrived</h2> | ||
| <!-- /wp:heading --> | ||
|
|
||
| <!-- wp:paragraph {"style":{"typography":{"fontSize":"16px"}}} --> |
There was a problem hiding this comment.
This is another place we use a fixed value. I think a preset value is more versatile for a pattern.
|
Shoot, a really great catch in i18n Tung! I've merged the other patterns but I'll update there too. |
dinhtungdu
approved these changes
Dec 5, 2022
|
Failed test is a flaky test and should be fixed by #7839. |
nerrad
force-pushed
the
add/just-arrived-full-hero
branch
from
28dc74c to
eebb596
Compare
|
If I don't need woocommerce patterns at all, is there any way to hide them from admin? |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The "Just Arrived Full Hero" commerce adjacent patterns is a straightforward hero block that can be used as inspiration for merchants on the home page to showcase an attention grabber image with simple callout text, and button block link to whatever page they want.
cc @vivialice for design review.
NOTE: The frontend screenshots were with images that won't ship with this pattern.
Screenshots
The following screenshots were with the Twenty Twenty Three theme and the "Whisper" style.
Editor
Pattern preview in Editor
Desktop frontend
Mobile frontend
Tablet frontend
Other Checks
Testing
Automated Tests
User Facing Testing
3. Verify the pattern inserted on the page. It may look different depending on the themeWooCommerce Visibility
Performance Impact
Changelog