Fix: Color problems with the newsletter sign-up pattern #528

juanfra · 2024-10-09T08:42:40Z

Description

Fixes #526

Update cover blocks to use contrast color for texts to work in different variations.

Screenshots

Screen.Recording.2024-10-09.at.10.40.14.mov

Testing Instructions

Create a page.
Insert the "Newsletter sign-up" pattern
Test the page with the different style variations.
Confirm that it looks good and the colors follow a11y guidelines.

… in different variations.

github-actions · 2024-10-09T08:42:53Z

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: juanfra <juanfra@git.wordpress.org>
Co-authored-by: carolinan <poena@git.wordpress.org>
Co-authored-by: beafialho <beafialho@git.wordpress.org>
Co-authored-by: sudip-md <sudipatel007@git.wordpress.org>

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

github-actions · 2024-10-09T08:42:56Z

Preview changes

You can preview these changes by following the link below:

Preview changes for twentytwentyfive

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.

beafialho · 2024-10-09T09:31:48Z

I'm trying to test this, but I'm having some issues with my local environment...

beafialho · 2024-10-09T09:43:38Z

Is this one looking unaccessible to you as well?

juanfra · 2024-10-09T10:06:44Z

Is this one looking unaccessible to you as well?

@beafialho yes, that's "Sunrise" right?

carolinan · 2024-10-09T10:08:33Z

The only two color contrasts that are guaranteed to work on all palettes are contrast and base.

beafialho · 2024-10-09T10:22:53Z

Is this one looking unaccessible to you as well?

@beafialho yes, that's "Sunrise" right?

Correct.

carolinan · 2024-10-09T10:45:29Z

From the issue:

Another important thing to consider is that I believe the color is meant to be the same as in the (stories) footer?

Because the stories footer and the newsletter sign-up has the same background color.

juanfra · 2024-10-09T14:17:23Z

Then, let's consider using the group block with Style 3 instead of the cover block. I will adjust the pattern for that.

carolinan · 2024-10-09T14:19:51Z

Great point, I am not sure why the cover was used.

juanfra · 2024-10-09T17:44:07Z

@beafialho @carolinan this one is ready for a new pass when you have time. Thanks 🙌

carolinan · 2024-10-10T04:33:27Z

patterns/cta-newsletter.php

-	<!-- wp:heading {"textAlign":"center","fontSize":"xx-large"} -->
-	<h2 class="wp-block-heading has-text-align-center has-xx-large-font-size"><?php esc_html_e( 'Sign up to get daily stories', 'twentytwentyfive' ); ?></h2>
-	<!-- /wp:heading -->
+<!-- wp:group {"tagName":"aside","metadata":{"categories":["call-to-action"],"patternName":"twentytwentyfive/cta-newsletter","name":"Newsletter Sign Up"},"align":"full","className":"is-style-section-3","style":{"spacing":{"padding":{"right":"var:preset|spacing|50","left":"var:preset|spacing|50","top":"var:preset|spacing|50","bottom":"var:preset|spacing|50"}},"dimensions":{"minHeight":""}},"layout":{"type":"constrained","contentSize":"800px"}} -->


Nit: remove metadata.

I believe we need to keep the categories so that the shuffle functionality can be more robust.

Have we concluded whether we're keeping the pattern name and name? I saw some discussions going on.

But the category, slug, name everything is listed in the file header.
Who can give a definite answer?

I could be getting it wrong, but it seems that the suffle functionality relies on the metadata. Please check this code.

The metadata is inserted in the editors when you insert the pattern.
So it is not missing from the shuffle functionality.

I went down a rabbit hole trying to find any official documentation, dev note or similar.

Sounds good, thanks for that. Let's try to define what we do with the metadata in the issue and work on it in bulk.

carolinan

From my end, using the section style on the group works much better across all the combined style variations.

sudip-md · 2024-10-10T06:47:43Z

Test Report

Description:
There is still a Midnight style background without a contrast color for the text. Everything else looks good.
Suggested : Midnight style (#4433A6)should have White font color.

Video -

2024-10-10_12-04-27.mp4

juanfra · 2024-10-10T07:33:20Z

@sudip-md thank you. That's not how I see it in the branch where I worked. This is how I see it:

Screen.Recording.2024-10-10.at.09.30.53.mov

Can you please check that you're on the update/newsletter-pattern branch, have pulled the latest changes, and re-inserted the pattern once you pulled the changes?

carolinan · 2024-10-10T11:34:11Z

I think this can be merged but we need to define the questions clearly if we can find who to reach out to about the metadata, including the translation of the metadata.

juanfra · 2024-10-10T11:40:58Z

I think this can be merged but we need to define the questions clearly if we can find who to reach out to about the metadata, including the translation of the metadata.

Sounds good. Shall we keep the conversation about metadata here? I saw they pinged Daniel Richards and maybe he has some more insights about it.

juanfra · 2024-10-10T15:05:22Z

I've removed the metadata and merging this one. In case we decide to add metadata to patterns we can revisit later in batch. Thank you 🙌

Update cover blocks to use contrast color for texts, so that it works…

9241ae4

… in different variations.

juanfra added Accessibility (a11y) Needs accessibility testing or feedback [Component] Block Patterns labels Oct 9, 2024

juanfra requested review from carolinan and beafialho October 9, 2024 08:42

juanfra self-assigned this Oct 9, 2024

juanfra added the [Type] Bug An existing feature does not function as intended. label Oct 9, 2024

Update pattern to use group with Style 3

6ed4822

carolinan reviewed Oct 10, 2024

View reviewed changes

carolinan mentioned this pull request Oct 10, 2024

Color problems with the Instagram grid pattern #468

Closed

carolinan approved these changes Oct 10, 2024

View reviewed changes

Remove metadata.

a0aa9a4

juanfra merged commit cdc72e2 into trunk Oct 10, 2024
4 checks passed

juanfra deleted the update/newsletter-pattern branch October 10, 2024 15:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix: Color problems with the newsletter sign-up pattern #528

Fix: Color problems with the newsletter sign-up pattern #528

juanfra commented Oct 9, 2024 •

edited

Loading

github-actions bot commented Oct 9, 2024 •

edited

Loading

github-actions bot commented Oct 9, 2024 •

edited

Loading

beafialho commented Oct 9, 2024

beafialho commented Oct 9, 2024

juanfra commented Oct 9, 2024

carolinan commented Oct 9, 2024

beafialho commented Oct 9, 2024

carolinan commented Oct 9, 2024

juanfra commented Oct 9, 2024

carolinan commented Oct 9, 2024 •

edited

Loading

juanfra commented Oct 9, 2024

carolinan Oct 10, 2024

juanfra Oct 10, 2024

carolinan Oct 10, 2024 •

edited

Loading

juanfra Oct 10, 2024

carolinan Oct 10, 2024 •

edited

Loading

juanfra Oct 10, 2024

carolinan left a comment

sudip-md commented Oct 10, 2024 •

edited

Loading

juanfra commented Oct 10, 2024

carolinan commented Oct 10, 2024

juanfra commented Oct 10, 2024

juanfra commented Oct 10, 2024

Fix: Color problems with the newsletter sign-up pattern #528

Fix: Color problems with the newsletter sign-up pattern #528

Conversation

juanfra commented Oct 9, 2024 • edited Loading

github-actions bot commented Oct 9, 2024 • edited Loading

github-actions bot commented Oct 9, 2024 • edited Loading

Preview changes

beafialho commented Oct 9, 2024

beafialho commented Oct 9, 2024

juanfra commented Oct 9, 2024

carolinan commented Oct 9, 2024

beafialho commented Oct 9, 2024

carolinan commented Oct 9, 2024

juanfra commented Oct 9, 2024

carolinan commented Oct 9, 2024 • edited Loading

juanfra commented Oct 9, 2024

carolinan Oct 10, 2024

Choose a reason for hiding this comment

juanfra Oct 10, 2024

Choose a reason for hiding this comment

carolinan Oct 10, 2024 • edited Loading

Choose a reason for hiding this comment

juanfra Oct 10, 2024

Choose a reason for hiding this comment

carolinan Oct 10, 2024 • edited Loading

Choose a reason for hiding this comment

juanfra Oct 10, 2024

Choose a reason for hiding this comment

carolinan left a comment

Choose a reason for hiding this comment

sudip-md commented Oct 10, 2024 • edited Loading

Test Report

juanfra commented Oct 10, 2024

carolinan commented Oct 10, 2024

juanfra commented Oct 10, 2024

juanfra commented Oct 10, 2024

juanfra commented Oct 9, 2024 •

edited

Loading

github-actions bot commented Oct 9, 2024 •

edited

Loading

github-actions bot commented Oct 9, 2024 •

edited

Loading

carolinan commented Oct 9, 2024 •

edited

Loading

carolinan Oct 10, 2024 •

edited

Loading

carolinan Oct 10, 2024 •

edited

Loading

sudip-md commented Oct 10, 2024 •

edited

Loading