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

TT1 Blocks: Add comments.php #157

Merged
merged 3 commits into from
Jan 11, 2021
Merged

TT1 Blocks: Add comments.php #157

merged 3 commits into from
Jan 11, 2021

Conversation

carolinan
Copy link
Collaborator

Follow up on #68
I consider this a temporary solution until a comment query for the individual post comment blocks is implemented.

Follow up on #68
I consider this a temporary solution until a comment query for the individual post comment blocks is implemented.
@carolinan
Copy link
Collaborator Author

TT1-blocks-nested-comments

@kjellr
Copy link
Collaborator

kjellr commented Jan 5, 2021

Thanks for finding this — I wasn't aware this was missing.

Similar to the CSS issues we've been encountering (the "Blocked" column here), my inclination would be to wait and leave this broken until it's supported natively via Gutenberg blocks. I'm curious what others think too.

@kjellr kjellr requested a review from scruffian January 5, 2021 13:40
@scruffian
Copy link
Collaborator

I agree with @kjellr, I'd rather log a blocking issue for this in Gutenberg.

@carolinan
Copy link
Collaborator Author

carolinan commented Jan 5, 2021

It comes back to what the purpose of the theme is. Is the purpose to make it easier to discover and test full site editing, or to highlight what has not been completed yet by allowing the theme to display PHP warnings and have missing features?

@pattonwebz
Copy link
Member

Some parts of FSE are still rough around the edges and needs additional work on the features to make FSE become more mainstream. If the theme is being released soon-ish then it will likely be released with some bugs/issues that need handled by the underlaying system in the future. It probably will throw PHP warnings/errors for users in some situations as well.

I think the 2 options here are:

  • Release it broken
  • Put the project on hold till the work on editor side is completed

I am not a fan of the idea of releasing a theme in the directory that is only parially functional - themes in the directory should be working and usable for users of all skill levels. A theme that is based on a core theme with a name so similar to a core theme and is likely to be available from the same username/author as the other core themes should really be showing off things in a good light rather than showing things to be not yet ready.

Can the theme exist in this repo here (or in it's own repo) untill GB handles the underlaying things the theme needs and that editor version is rolled into and released with core?

@carolinan
Copy link
Collaborator Author

Removing the CSS but keeping the comments.php file and the comments form block would show the comments without PHP issues.

image

@kjellr
Copy link
Collaborator

kjellr commented Jan 5, 2021

Is the purpose to make it easier to discover and test full site editing, or to highlight what has not been completed yet by allowing the theme to display PHP warnings and have missing features?

I can only speak for myself here, but this has always been my understanding of the purpose, based on chats with @mtias:

TT1 Blocks is an attempt to build a replica of the existing Twenty Twenty-One theme, solely using the native full-site editing toolset. The theme will be “complete” when that is achieved. In the meantime, the primary aspect of this goal is to discover and prioritize areas where the editor needs to catch up (like those blocked areas in the project board). A secondary aspect is to allow folks to test full-site editing, though like full-site editing itself, I’d expect that the theme today would not be feature-complete for these users.

I am not a fan of the idea of releasing a theme in the directory that is only parially functional - themes in the directory should be working and usable for users of all skill levels. A theme that is based on a core theme with a name so similar to a core theme and is likely to be available from the same username/author as the other core themes should really be showing off things in a good light rather than showing things to be not yet ready.

There are two other block-based themes live in the repo today: 

In the interest of minimizing code that we'll delete later on (and not including functionality that purely-block-based themes do not support yet), I'd be happy adopting either of those approaches here.

@carolinan
Copy link
Collaborator Author

Except block based themes do support comments. The post comments block is fully functional by including wp_list_comments().

What is not yet implemented is using a loop/query of all comments, with the individual post comment blocks inside.
These blocks only work inside the post comment block, with a comment ID defined.

@pattonwebz
Copy link
Member

There are two other block-based themes live in the repo today: 

In the interest of minimizing code that we'll delete later on (and not including functionality that purely-block-based themes do not support yet), I'd be happy adopting either of those approaches here.

Either option would be fine with me too in the interest of getting the theme to it's goal of showcasing what is possible and what is still needing worked on at the editor side.

Seems like @carolinan knows a solution that allows comments to work though and would result in only a minimal change in future should editor be adjusted to allow querying individual comment blocks.

@kjellr
Copy link
Collaborator

kjellr commented Jan 5, 2021

What is not yet implemented is using a loop/query of all comments, with the individual post comment blocks inside.
These blocks only work inside the post comment block, with a comment ID defined.

Is there a Gutenberg issue for this? I couldn't find one just now, but it seems like this falls into the category of things that would be great to prioritize upstream.

@carolinan
Copy link
Collaborator Author

-There aren't even labels for these blocks so it is very difficult to find issues if they exist.

I did not understand the reasoning behind why the inner blocks were added this way, because, there are not so many use cases for displaying a single comment using an ID.

@carolinan
Copy link
Collaborator Author

This is probably the one, I added the post comments label to it meanwhile WordPress/gutenberg#24101
The post comment and the post comments block are two different blocks.

@scruffian
Copy link
Collaborator

I did not understand the reasoning behind why the inner blocks were added this way, because, there are not so many use cases for displaying a single comment using an ID.

Inner blocks are useful for arranging the content; so even if they aren't used on their own, it's useful to have these as separate blocks to take advantage of the block controls and the layouts this enables.

@carolinan
Copy link
Collaborator Author

It is about as useful as watercolors without a brush or a paper to paint on. You need all three.


Arguing for a broken theme and that we should not include an improvement because we need to update it later on is
moot because most of the theme will need continuous updates.
-When the blocked issues are solved, the theme still needs to be updated to work with those changes.

Adding the file is not preventing further work on the comments block in any way.
-The only thing standing in the way of the comments block is having people actually working on it.

The theme directory is not a development repository.
The theme needs to follow the theme review requirement and it needs to be complete.
Does it need to be pretty and include CSS for everything to be in the directory? No, it doesn't, but it needs to be as complete as it can be.

I don't have the privilege to be able to look at the theme and see one theme.
But I do have a bigger responsibility to maintain the theme and the theme directory than others.

I have to look at the bigger picture where in 3 weeks, 3 months and 12 months, there are going to be theme authors that will look at this theme and they are going use the exact same arguments:
"But they (Q, Bosco) were allowed to submit a theme that does XXX why aren't we?"
Neither full site editing or the theme directory will benefit from that.

If it is unclear, I am trying to improve the theme. I have nothing to gain from delaying the release of the theme, that is not what I am trying to do. And I also don't have access to packaging and submitting the theme, all I can do is help prepare it.

@scruffian
Copy link
Collaborator

My understanding was that the purpose of putting the theme in the theme directory was to enable more people to test Full Site Editing.

From my perspective the aim of TT1 Blocks is to:

  • Demonstrate what is possible currently with Full Site Editing
  • Make clear what is still lacking in Full Site Editing, so that work on Full Site Editing can be prioritized such that it fills these gaps.

As we have seen with Varia and Seedlet, when a theme plugs gaps in the editor with CSS, this creates confusion around where code should live, what issues there are unresolved in the editor, issues when the editor plugs these gaps and then the theme breaks etc. It also lowers the priority of these things so they don't get worked on.

I would rather not put the theme into the directory until it was ready, than start plugging gaps in the editor via theme CSS.

@carolinan
Copy link
Collaborator Author

As I wrote in #157 (comment)
Removing the CSS but keeping the comments.php file and the comments form block would show the comments without PHP issues.

@kjellr
Copy link
Collaborator

kjellr commented Jan 7, 2021

I've opened a separate issue to track this specific bug: WordPress/gutenberg#28045

Let's hold off for a day or so and see if that leads to any promising developments. If not, I think it's ok to temporarily add this code to suppress the error and move forward with the initial submission to the theme repo. Does that seem reasonable to everyone?

@carolinan
Copy link
Collaborator Author

I opened an issue for this in may ;) But your description was better.

@carolinan carolinan changed the title TT1 Blocks: Add comments.php and comment area styles TT1 Blocks: Add comments.php Jan 7, 2021
@kjellr
Copy link
Collaborator

kjellr commented Jan 11, 2021

There was no movement on that issue, so I'll merge this for now. I've added a quick code comment to reference the Gutenberg issue. I'll get us started on submitting to the theme repository later today!

Thank you!

@kjellr kjellr merged commit d33eeeb into master Jan 11, 2021
@kjellr kjellr deleted the tt1-comments branch January 11, 2021 14:28
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.

4 participants