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

Content updates from Page Editor #512

Merged
merged 2 commits into from
Nov 12, 2024
Merged

Content updates from Page Editor #512

merged 2 commits into from
Nov 12, 2024

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Nov 11, 2024

The following pages have been updated in the Page Editor.

WordPress 6.7 [https://wordpress.org/download/releases/6-7/]
Home [https://wordpress.org/]

Screenshots: https://github.com/WordPress/wporg-main-2022/actions/runs/11805493962/artifacts/2178654527

Please review, merge, and deploy.

@jasmussen
Copy link
Contributor

Hey, just noting I'm working on this page for the moment, to fix a few spacing, lineheight aspects, and upload new versions of the videos that do not feature intro text.
Screenshot 2024-11-11 at 10 24 14

@jasmussen
Copy link
Contributor

Okay, according to feedback starting here, and here, I've:

  • Updated the lineheight of the italic text
  • Adjusted spacings, paddings, to better collapse items on mobile (see screenshot above)
  • Uploaded versions of the videos without an intro

I'm seeing text-wrap: pretty correctly applied:

Screenshot 2024-11-11 at 10 38 59

However for some reason I'm still seeing this typographic widow here:
Screenshot 2024-11-11 at 10 38 15

@jasmussen jasmussen mentioned this pull request Nov 11, 2024
@github-actions github-actions bot force-pushed the automated/content-update branch 2 times, most recently from 69dab3c to 58bad94 Compare November 11, 2024 12:35
@ryelle
Copy link
Contributor

ryelle commented Nov 11, 2024

Line height definitely looks better 👍🏻

I'm also correctly seeing "pretty" work in Chrome, are you on Firefox for that screenshot? Firefox does not support pretty. It's also a browser-dependent algorithm for longer blocks of text, so perhaps it's not detecting the right place on this short line?
localhost_8888_download_releases_6-7_

It also looks like the group blocks are missing some padding in Firefox:

Screenshot 2024-11-11 at 4 20 51 PM

This same visual happens when you click play in Chrome, the group height shrinks:

ScreenFlow.mp4

Should the pink video (block bindings) have been updated too? it still has the intro text.

@github-actions github-actions bot force-pushed the automated/content-update branch from 58bad94 to 5521c5b Compare November 11, 2024 21:52
@ryelle
Copy link
Contributor

ryelle commented Nov 11, 2024

Ah, I think the size change issue is due to the poster image being a different aspect ratio than the video itself — an image 1200 × 675 would be better, to prevent that resize jump.

I've adjusted the padding on the yellow & pink text columns and that fixes the spacing issue, and doesn't introduce too much extra spacing on small screens.

Screen Shot 2024-11-11 at 16 50 22

On small screens on Chrome, there is extra space due to the square poster image, if that can be updated, FF & Chrome should match.

Firefox Chrome
Screenshot 2024-11-11 at 4 52 09 PM Screenshot 2024-11-11 at 4 52 58 PM

@github-actions github-actions bot force-pushed the automated/content-update branch from 5521c5b to e8dcbfa Compare November 11, 2024 22:05
@jasmussen
Copy link
Contributor

I'm also correctly seeing "pretty" work in Chrome, are you on Firefox for that screenshot? Firefox does not support pretty. It's also a browser-dependent algorithm for longer blocks of text, so perhaps it's not detecting the right place on this short line?

Yeah it looks like any shortcomings here are beyond our control. Thank you for looking.

Taking a stab at the rest of your feedback now.

@jasmussen
Copy link
Contributor

Excellent notes on Firefox. For now I've saved a change that adds a custom CSS class, has-1-1-aspect-ratio, to each video block. Combined with this custom CSS (can you add this?) it works for me in Firefox:

.has-1-1-aspect-ratio video {
    aspect-ratio: 1/1;
}

Screenshot 2024-11-12 at 08 28 39

I'll take a look at the video files intro piece now!

@jasmussen
Copy link
Contributor

Okay, the intro text from the purple video is gone. I uploaded the wrong file yesterday. Long day!

@jasmussen
Copy link
Contributor

As a final note, if we add the custom CSS mentioned here, it should also fix the differential you're seeing in the mobile view, and make FF/Chrome match:
Screenshot 2024-11-12 at 08 34 18

I think that addresses all of it. Thanks for your help and feedback.

@github-actions github-actions bot force-pushed the automated/content-update branch from e8dcbfa to 52b3f99 Compare November 12, 2024 07:37
@ryelle
Copy link
Contributor

ryelle commented Nov 12, 2024

Combined with this custom CSS (can you add this?) it works for me in Firefox:

Can we fix this without custom CSS? is this a gutenberg bug?

@jasmussen
Copy link
Contributor

It's not a bug, it's just that the video block does not support the aspect ratio control. It should, and I'll open an issue. If it did, we could fix it.

@jasmussen
Copy link
Contributor

Opened an issue here: WordPress/gutenberg#66933

@ryelle
Copy link
Contributor

ryelle commented Nov 12, 2024

@jasmussen The aspect-ratio CSS cuts off the video once it's playing, is that okay? It seems strange especially on this video, since the sidebar is fully cut off.

Screenshot 2024-11-12 at 11 00 55 AM

@ryelle
Copy link
Contributor

ryelle commented Nov 12, 2024

I'm going to merge this without the CSS change, to avoid the cut-off videos. We can always iterate on this after it's live. I still think it would make more sense to update the poster image with the correct video aspect-ratio (or re-render the videos as squares, but I imagine that's a bigger task).

@jasmussen
Copy link
Contributor

Hmm that's curious indeed.

Yes, I'll circle back to this, I may need to apply the aspect ratio to a containing group instead of the video block directly. Thank you for the help so far!

@github-actions github-actions bot force-pushed the automated/content-update branch from 52b3f99 to e10ad43 Compare November 12, 2024 20:56
@ryelle
Copy link
Contributor

ryelle commented Nov 12, 2024

I've added the homepage update (#507) & updated the release page to use the page content pattern instead of the in-progress pattern, once 6.7 is out I'll merge & deploy this.

@ryelle ryelle merged commit 5391fab into trunk Nov 12, 2024
2 checks passed
@ryelle ryelle deleted the automated/content-update branch November 12, 2024 21:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants