Size Change: +1.46 kB (+0.06%)

Total Size: 2.57 MB

_{compressed-size-action}

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: youknowriad <[email protected]>
Co-authored-by: jasmussen <[email protected]>
Co-authored-by: colorful-tones <[email protected]>
Co-authored-by: carolinan <[email protected]>

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

There's a few style glitches that are related to the "Page" component and not the font library. These are going to require normalizing the padding in all site editor pages (all viewports too probably) to 24px. I can take a look at this separately.

Also related, the footer is not pushed to the bottom.

Nice, we want this. It's also intriguing on several levels.

There are a couple of little details, like misaligned padding and such here:

I tried a little web-inspecting to shore things up:

• Updated left/right padding for the top header. It's actually 16px now, because there's also an inner wrapper for just the title itself, but it comes to 24px left and right padding. Related: DataViews: Try using 24px padding for consistency across different uses #73334
• 16px margin outside the "frame", just like in the site editor. No reason to diverge here, IMO.
• 16px left/right padding on the tab-bar, so the text inside the tab lines up.

I sense there was an intention behind the padding of 8px, so an alternative we could do would be this:

That's the same 16px margin on the bottom and right, but zero margin left and top, to align flush with the menu and adminbar.

One more thing, the title is "gutenberg-boot", I assume that'll become "Fonts < [site name]".

This feedback is specific to the PR here in question. I do think there's a larger systematic approach for this, where we have only a single "Fonts" page, not several, but that requires a unification of concerns that is a larger undertaking. For that reason I'm assuming this is a situational change that brings the wonderful benefit of font management to the masses immediately, but that we are also able to consolidate pages at some point in the future. Is that possible?

Oh, and can we bottom align the footer?

@jasmussen check my previous comments :P I know about the spacing and alignment issues but they shouldn't be fixed adhoc here. There two main issues here and they both relate to the Page component and the fix will impact the site editor too.

One more thing, the title is "gutenberg-boot", I assume that'll become "Fonts < [site name]".

Good point, I didn't think about how to approach "page titles" in this new architecture yet. I'll have to think about it :) (and probably also address separately).

For now, consider this is a long running (I mean like a week) experiment but I'd like to land things in steps.

Ok I made some updates here, I think this is looking good enough to ship. WDYT.

Flaky tests detected in 4447533.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/20026540637
📝 Reported issues:

• [Flaky Test] should open and edit templates correctly when active_templates experiment is disabled #73743 in /test/e2e/specs/site-editor/template-id-format.spec.js

This is nice:

I still think you should do the padding changes I proposed here.

It's a "do or do not" thing: either we're embracing that this is a rounded frame, or we shouldn't have the frame at all. That translates to: have a visible padding, same as in the site editor. The present padding doesn't look intentional.

Oh, and I still don't see a page title. That's a followup yes?

16px left/right padding on the tab-bar, so the text inside the tab lines up.

Is this the padding thing that is missing? Because here I prefer to keep 24px because that's how all the tabs are aligned in http://localhost:8888/wp-admin/admin.php?page=gutenberg-boot so this would be inconsistent. But could be considered separately while keeping consistency.

For the page title, it first needs an API that is addressed in #73847

It's a "do or do not" thing: either we're embracing that this is a rounded frame, or we shouldn't have the frame at all. That translates to: have a visible padding, same as in the site editor. The present padding doesn't look intentional.

I don't understand this and this is intentional and matches the new direction we're taking in http://localhost:8888/wp-admin/admin.php?page=gutenberg-boot

Ah I see. One thing is the fonts page (this PR) the other ois that separate and very exciting view. On the latter link, the padding is consistent. See how the site editor has this padding:

That padding is just different in this context. But it is mainly different when this is embedded as a single view on the right, vs. the full thing.

No blockers, I guess we'll need to iterate this anyway. But I was specifically giving feedback on this PR.

It's supposed to be the same code, I'll check if there's a glitch in the single view, you need Joen's eagle eyes to see it.

Is there a reason why this page is not available for hybrid themes?

@mrleemon I'll take a look for now I started very conservative and I'll see about opening it more. Do "hybrid themes" have access to global styles today?

@youknowriad Yes, via a theme.json file

@mrleemon I mean in the UI, can you edit it?

I'd love an example of hybrid theme to test against btw, in case you know any theme in the repo that fits this description.

@mrleemon Consider testing this PR to see if addresses your needs

@youknowriad Will test and report back.