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

Update/pattern headings #543

Merged
merged 13 commits into from
Oct 11, 2024
Merged

Conversation

troychaplin
Copy link
Contributor

Description

To update patterns to have headings as per the comments in #408

Screenshots

Cover with big heading CV Bio Link in bio with tight margins

Copy link

github-actions bot commented Oct 10, 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: troychaplin <[email protected]>
Co-authored-by: carolinan <[email protected]>
Co-authored-by: juanfra <[email protected]>
Co-authored-by: beafialho <[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 Oct 10, 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.

<!-- /wp:paragraph -->
<!-- wp:heading {"textAlign":"left","style":{"typography":{"lineHeight":"1.2"}},"fontSize":"x-large"} -->
<h2 class="wp-block-heading has-text-align-left has-x-large-font-size" style="line-height:1.2"><?php esc_html_e( 'I&rsquo;m Asahachi Kōno, a Japanese&nbsp;photographer, a member of&nbsp;Los Angeles\'s Japanese Camera Pictorialists of California. Before returning to Japan, I worked as a photo retoucher.', 'twentytwentyfive' ); ?></h2>
<!-- /wp:heading -->
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not sure about this one. It does not feel like a heading above other content?

Copy link
Contributor Author

@troychaplin troychaplin Oct 10, 2024

Choose a reason for hiding this comment

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

Agreed, reverted in recent commit

<p class="has-text-align-left" style="font-size:22rem;letter-spacing:-0.03em"><?php echo esc_html_x( 'Hey,', 'Example heading above the content of the CV/Bio pattern.', 'twentytwentyfive' ); ?></p>
<!-- /wp:paragraph -->

<!-- wp:heading {"textAlign":"left","className":"wp-block-heading","style":{"typography":{"fontSize":"22rem","letterSpacing":"-0.03em","fontStyle":"normal","fontWeight":"300","lineHeight":"1.4"}}} -->
Copy link
Contributor

Choose a reason for hiding this comment

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

This custom className is not needed. It will show up in the additional CSS field in the editor.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fix in recent commit

<!-- wp:paragraph {"align":"left","style":{"typography":{"fontSize":"clamp(1rem, 380px, 24vw)","letterSpacing":"-0.02em","lineHeight":"1","fontWeight":"700"}}} -->
<p class="has-text-align-left" style="font-size:clamp(1rem, 380px, 24vw);font-weight:700;letter-spacing:-0.02em;line-height:1"><?php esc_html_e( 'Stories', 'twentytwentyfive' ); ?></p>
<!-- /wp:paragraph -->
<!-- wp:heading {"align":"left","className":"has-text-align-left","style":{"typography":{"fontSize":"clamp(1rem, 380px, 24vw)","letterSpacing":"-0.02em","lineHeight":"1","fontWeight":"700","fontStyle":"normal"}}} -->
Copy link
Contributor

Choose a reason for hiding this comment

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

The custom className is not needed. It will show up in the additional CSS field in the editor.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fix in recent commit

@beafialho
Copy link
Contributor

Thank you @troychaplin, rhese are looking good to me. Where these three the only patterns that had these changes?

@troychaplin
Copy link
Contributor Author

troychaplin commented Oct 10, 2024

Thank you @troychaplin, rhese are looking good to me. Where these three the only patterns that had these changes?

After better review of my notes and comments from @carolinan I believe these may be the only changes required. I made a couple notes here, not sure if either will require changes.

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.

Thank you @troychaplin

The aesthetics looks good, how it was. I left a couple of comments.

<!-- /wp:paragraph -->

<!-- wp:heading {"textAlign":"left","style":{"typography":{"fontSize":"22rem","letterSpacing":"-0.03em","fontStyle":"normal","fontWeight":"300","lineHeight":"1.4"}}} -->
<h2 class="wp-block-heading has-text-align-left" style="font-size:22rem;font-style:normal;font-weight:300;letter-spacing:-0.03em;line-height:1.4"><?php echo esc_html_e( 'Hey,', 'twentytwentyfive' ); ?></h2>
Copy link
Member

Choose a reason for hiding this comment

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

Can we keep the esc_html_x here so that there's context for the translators? Hey, is a text that can cause some confusion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changes push in latest commit

<!-- wp:heading {"textAlign":"left","style":{"typography":{"fontSize":"22rem","letterSpacing":"-0.03em","fontStyle":"normal","fontWeight":"300","lineHeight":"1.4"}}} -->
<h2 class="wp-block-heading has-text-align-left" style="font-size:22rem;font-style:normal;font-weight:300;letter-spacing:-0.03em;line-height:1.4"><?php echo esc_html_e( 'Hey,', 'twentytwentyfive' ); ?></h2>
<!-- /wp:heading -->
Copy link
Member

Choose a reason for hiding this comment

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

Can we fix this? It looks like there's an empty space.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changes push in latest commit

@juanfra juanfra added the [Type] Enhancement A suggestion for improvement. label Oct 10, 2024
@carolinan carolinan merged commit 7b2f3d4 into WordPress:trunk Oct 11, 2024
5 of 7 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
[Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants