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

Full page patterns: Improve zoom out #514

Open
wants to merge 6 commits into
base: trunk
Choose a base branch
from
Open

Conversation

carolinan
Copy link
Contributor

@carolinan carolinan commented Oct 8, 2024

Description

This PR updates the block patterns that are designed as full page layouts. These patterns were built using one group that wrapped all inner patterns. The PR removes the outer group and instead places each pattern inside its own group.
These new groups have top and bottom margin set to 0, to remove the blank space between the sections.

This ensures that each inner pattern show as a separate section when zoom out is enabled, and the spacing is maintained.

Closes #497

Screenshots

zoom-out-3-oct-8.mp4

Testing Instructions

Create a new page.
In the pattern selection modal, select a pattern that has multiple sections, for example the business homepage.
Confirm that visually, there is no change before and after the PR.
Select and add background colors to all patterns: Confirm that there is no vertical space between the colored sections.
Enable zoom out. Confirm that you can add patterns between the sections.

  • Repeat for all patterns updated by this PR.

Copy link

github-actions bot commented Oct 8, 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.

@carolinan carolinan added [Type] Enhancement A suggestion for improvement. [Priority] High Used to indicate top priority items that need quick attention [Component] Block Patterns labels Oct 8, 2024
@carolinan carolinan marked this pull request as ready for review October 8, 2024 07:07
Copy link

github-actions bot commented Oct 8, 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: carolinan <[email protected]>
Co-authored-by: juanfra <[email protected]>
Co-authored-by: jasmussen <[email protected]>
Co-authored-by: richtabor <[email protected]>
Co-authored-by: getdave <[email protected]>
Co-authored-by: qasumitbagthariya <[email protected]>
Co-authored-by: youknowriad <[email protected]>
Co-authored-by: annezazu <[email protected]>
Co-authored-by: MaggieCabrera <[email protected]>

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

@carolinan
Copy link
Contributor Author

carolinan commented Oct 8, 2024

I was not able to get a recording of this where the patterns don't show outside the zoom out area. Deactivating zoom and and enabling it again still shows the pattern outside the area, but by a smaller amount.

@jasmussen
Copy link
Contributor

Tested the outlined patterns, this is working well for me. Here's just one example:

test

Separate from this PR, we probably need to tweak the Stories pattern, this one:

Screenshot 2024-10-08 at 09 43 48

It seems to break onto two lines in very many cases, unfortunately, so we might need to reduce the font size a bit.

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 Carolina, it's working well for me. I was able to use the zoom out mode with these landing pages, the gap is not there and I'm able to insert patterns in between.

One thing I noticed, probably unrelated to this PR and a problem with zoom out is that when the list view or inserter sidebar are not there, the zoom out has some sort of left/right padding. I'm on the latest Gutenberg (trunk)

Screen.Recording.2024-10-08.at.15.42.41.mov

@jasmussen
Copy link
Contributor

@getdave would you be able to briefly take a look at juanfra's comment above, see if this is a known issue in the block editor or not?

@richtabor
Copy link
Member

It's not 100% ideal, but it works. Can we add names to these groups though, so it's more helpful to scan and identify patterns?

CleanShot 2024-10-08 at 23 57 35

@carolinan
Copy link
Contributor Author

@richtabor The problem with the names is that it was discovered that they are not translatable.

@getdave
Copy link

getdave commented Oct 9, 2024

@richtabor The problem with the names is that it was discovered that they are not translatable.

Oh that's not good.

The names are stored in the metadata.name block attribute. Is there an Issue tracking that problem?

@getdave
Copy link

getdave commented Oct 9, 2024

Oh hold on. These are .php files. So cant we just use PHP to add the attributes.metadata.name and run it through a translation function at point of output?

<!-- wp:group {"metadata":{"name":"<?php _e( 'My custom name here', 'my-text-domain' ) ?>"},"align":"full","className":"is-style-section-5","style":{"spacing":{"padding":

@carolinan
Copy link
Contributor Author

It needs to be tested. Please see the linked conversation here: #403

@carolinan
Copy link
Contributor Author

carolinan commented Oct 9, 2024

There are already attributes in other pattern files that have the PHP functions added inside, like the aria labels for the search block.

@richtabor
Copy link
Member

richtabor commented Oct 9, 2024

It needs to be tested. Please see the linked conversation here: #403

Left a comment there. We should really consider adding names to these—they're much more identifiable in list view with names.

@qasumitbagthariya
Copy link

Upon thoroughly reviewing on my end, everything appears to be in order. The zoom functionality is working as expected, including proper interaction with the group name.

Screen.Recording.2024-10-11.at.5.52.52.PM.mov

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Component] Block Patterns [Priority] High Used to indicate top priority items that need quick attention [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Zoom out is not working on full page patterns
6 participants