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

When a block style is registered for the query, the "Create a new post for this feed." link is in the Style section #49327

Closed
carolinan opened this issue Mar 24, 2023 · 11 comments · Fixed by #49819
Assignees
Labels
[Block] Query Loop Affects the Query Loop Block [Type] Bug An existing feature does not function as intended

Comments

@carolinan
Copy link
Contributor

Description

When you register a custom block style for the query (with register_block_style or the JavaScript equivalent) the link to where you add a new post to the query is in Styles section.

This is a bit difficult to explain without the screenshot:
When you open the block settings sidebar,
The link to add new posts is below the title "Styles" and below the buttons that you use to select the style variation.

Step-by-step reproduction instructions

Register any block style for the query loop.
My example looks like this:

<?php
/**
 * Register block styles.
 */
function blue_note_register_block_styles() {
	register_block_style(
		'core/query',
		array(
			'name'  => 'blue-note-query-slant',
			'label' => __( 'Slanted images', 'blue-note' ),
			'inline_style' => '.is-style-blue-note-query-slant .wp-block-post-featured-image {transform: rotate(-1deg);}',
		)
	);
}
add_action( 'init', 'blue_note_register_block_styles' );

Open the block editor.
Add a query block.
View the Styles section in the block settings sidebar.

Screenshots, screen recording, code snippet

Missplaced link in the styles section

Environment info

No response

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

@carolinan carolinan added [Block] Query Loop Affects the Query Loop Block [Type] Bug An existing feature does not function as intended labels Mar 24, 2023
@carolinan
Copy link
Contributor Author

carolinan commented Mar 25, 2023

I did not find a good way to move the hooked link and text.

  1. The link can be placed inside its own panelbody, but the panel would still be below the Styles, not above, and it doesn't look very good.
  2. Another option would be to enable the styles tab for this block, even though it does not have style block supports, and have the link display at the top of the settings tab and the style variations in the styles tab.

@t-hamano
Copy link
Contributor

t-hamano commented Apr 2, 2023

To solve this problem, I suggest either of the following approaches:

Add a new slot to the block card content

Add a new Slot at the bottom of the BlockCard component (or in the description). This ensures that the content is displayed immediately below the block description, unaffected by InspectorControls. It would also help third-party developers add links about the block.

block-card-content

Rendering BlockStyles panel via Slot

The block style panel is hard-coded just before the InspectorControls slot. Like the Layout panel, if rewritten using InspectorControls, the user's own added InspectorControls should be rendered before the BlockStyles panel.


cc: @aaronrobertshaw

@carolinan
Copy link
Contributor Author

Maybe this link could be removed?

@aaronrobertshaw
Copy link
Contributor

Are we sure that the Query block should have Block Styles?

In some recent discussions around the Query block. There was a general consensus that the block shouldn't be a style provider, that is something that drove the removal of the color block supports from the Query block (#46147). Styles were to be moved to inner blocks such as the PostTemplate block in the Query block's case here.

Some further context and history can be found in:

The objections to adding a Slot to the BlockCard in #45437 essentially rule out the first of @t-hamano's suggestions.

I'll take a look at introducing an internal use only Slot for the Block Styles to be rendered into in the Block Inspector.

@carolinan
Copy link
Contributor Author

Great question, what would it take to remove it only for this single block?
As developers we can always work around the CSS hierarchy, but removing existing features does create friction. How do we make sure that a user with an existing style does not end up with a broken style, or is not able to remove a style that is already applied?

@aaronrobertshaw
Copy link
Contributor

If a block style has previously been added to the Query block, any blocks that have selected it would have the classname in its markup e.g. is-style-<name>. If the block style was removed then that class name would be caught and added to the block's custom className attribute.

A deprecation is usually used in this situation to migrate blocks. As this is something being added to the Query block outside of core, I imagine that a deprecation would need to be added as well. Perhaps the blocks.registerBlockType filter could provide a means of achieving that.

That said, deprecations will only migrate blocks as they are loaded in the editor and require the edited post to be saved. As such, the class name on the Query block would need to continue to be supported.

@carolinan
Copy link
Contributor Author

I guess it depends on if only the visibility of the setting is disabled, or the entire feature, since the CSS can be proved in the registration function, not only from a stylesheet that the theme enqueues.

@carolinan
Copy link
Contributor Author

If a developer continues to use register_block_style with the block, they may be confused when the block style "stops working" (stops being a selectable block option) and there is no "doing it wrong" message...

@aaronrobertshaw
Copy link
Contributor

If a developer continues to use register_block_style with the block, they may be confused when the block style "stops working" (stops being a selectable block option) and there is no "doing it wrong" message...

I don't think we can force Query to "turn off" Block Styles. I was more proposing that we discourage their use a little for a "settings-only" block like the Query block.

Ultimately, it would be up to the 3rd party developers that had introduced the block styles here, to relocate them, and add deprecations. I don't think we can count on that, especially if they may disagree with the concept of settings-only blocks and using something like the Post Template or Group blocks as a style provider.

It would be best if we could address the location of the create new post link anyway. As noted above, it's something I'm looking into at the moment.

I'll take a look at introducing an internal use only Slot for the Block Styles to be rendered into in the Block Inspector.

I'll update here when I have a potential fix ready.

@aaronrobertshaw
Copy link
Contributor

Fix is up in #49566.

Opted not to go with a new slot as even using the private-apis package, we can't 100% enforce that it isn't abused.

@aaronrobertshaw
Copy link
Contributor

Turns out we might be better served with an alternate approach in #49819. Either way, a fix for this should land soon.

@priethor priethor removed the [Status] In Progress Tracking issues with work in progress label May 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Query Loop Affects the Query Loop Block [Type] Bug An existing feature does not function as intended
Projects
None yet
4 participants