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

WIP: Add query description block #29609

Closed
wants to merge 1 commit into from

Conversation

carolinan
Copy link
Contributor

Description

How has this been tested?

Screenshots

Types of changes

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • I've tested my changes with keyboard and screen readers.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR.

Copy link
Contributor

@ntsekouras ntsekouras left a comment

Choose a reason for hiding this comment

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

Thanks for working on this @carolinan !

I have thought about this (not in depth though) when working on Query Title: #29428 and #27989.

TLDR: This block and Query Title should probably handle more cases, which are not identified yet, with the exception of a Search Title and probably a Search description (?).

I think there are two alternatives here:

  1. Use block variations here as well
  2. I was thinking that we might be able to just use Query Title and have an option to show description, based on its variation.

What do you think?

export const settings = {
title: _x( 'Query Description', 'block title' ),
description: __(
'Displays a description for the term.'
Copy link
Contributor

Choose a reason for hiding this comment

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

This is very specific to term queries.

@ntsekouras ntsekouras mentioned this pull request Mar 8, 2021
7 tasks
@carolinan
Copy link
Contributor Author

I don't think there is anything like a search description?

@carolinan
Copy link
Contributor Author

Also if these are not separate blocks, then it will be much more difficult to handle alignments, font sizes, colors, line height, padding and whatever else we can come up with.
Because you would need to have different controls for the different elements. This has been mentioned before but no solution has been suggested as far as I know.

@aristath
Copy link
Member

aristath commented Mar 8, 2021

Currently, there is no such thing as a Query Description. Since this is very specific to term-descriptions maybe rename it to that so it's clear what it does? It will avoid a lot of confusion.

@ntsekouras I don't understand what the title has to do with term descriptions... 🤔 Can you please elaborate?

@carolinan
Copy link
Contributor Author

carolinan commented Mar 8, 2021

I can't come up with a scenario where the description would be used without the query block - or without the title 🤔

For example if a shop plugin would display different products below a description, it would still be using a query, no?

@ntsekouras
Copy link
Contributor

ntsekouras commented Mar 8, 2021

@ntsekouras I don't understand what the title has to do with term descriptions... 🤔 Can you please elaborate?

When I started working on Archive Title it was pointed out that might be other use cases for Query Title. After asking and waiting for feedback, the only one provided, that makes sense, is Search Title. I haven't thought too much about the description, but thought there might be other similar cases for Description. I don't have something specific in mind. If you agree that there is not another case for Query description, then we should go with something specific as Ari mentioned ( Term description.

Also if these are not separate blocks, then it will be much more difficult to handle alignments, font sizes, colors, line height, padding and whatever else we can come up with.

Agree.

@carolinan
Copy link
Contributor Author

Anyway, no matter what block that is used, where I really got stuck was when I tried to find a way to display the correct text, and not a placeholder, in the editor.

@aristath
Copy link
Member

aristath commented Mar 8, 2021

In the alternative PR on #29613 the block can be used outside a query... So you can have for example an archive.html file in the theme with the following structure:

{TERM NAME HERE}
<!-- wp:term-description /-->
<!-- wp:query {"queryId":0,"query":{"inherit":true}} -->
.....

The term-description doesn't need to be inside the query itself, it's a generic block.

@carolinan
Copy link
Contributor Author

That works on the front, but not the editor, unless you also want an option where you select which term to show the description for.
Without that, the block can not be editable.

@aristath
Copy link
Member

aristath commented Mar 8, 2021

Without that, the block can not be editable.

But it doesn't need to be editable... At least not for an MVP - which is what we're trying to accomplish. The block can show a placeholder on the editor and the proper description on the frontend. Not everything needs to be editable on the site-editor....

@ntsekouras
Copy link
Contributor

But it doesn't need to be editable... At least not for an MVP

I agree with that. That's what I have for Archive Title as well.

@ntsekouras
Copy link
Contributor

Thanks for your work Carolina and your help with the other PR and Query Title 💯

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.

3 participants