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

Exploring blocks for displaying coauthors #932

Closed
douglas-johnson opened this issue Mar 7, 2023 · 8 comments
Closed

Exploring blocks for displaying coauthors #932

douglas-johnson opened this issue Mar 7, 2023 · 8 comments

Comments

@douglas-johnson
Copy link
Contributor

douglas-johnson commented Mar 7, 2023

I am interested in displaying coauthor data in block editor and full site editing contexts.

I started a branch on a fork where I put together two blocks for this purpose. Here is a link to compare it to the current master version: master...thoughtis:Co-Authors-Plus:block-exploration

I am interested in feedback from both users and maintainers. Depending on the feedback I am happy to either submit a pull request, or keep this as a separate project. Thanks for taking a look.

What it does

  • There is a coauthors block which works like the Post Template block.
    • It uses the postId context and fetches the coauthors.
    • Then it allows you to create a repeating template for displaying the coauthors.
  • The first available block within that template is the coauthor-display-name block.
    • It pretty simply shows the Display Name based on the provided context.
  • You can use these within the context of the current post you're editing, or with the Query Loop block.

Example

Screenshot of the editor where I'm outputting the display name of the authors in both the current post and a query loop. The stored and rendered versions of this post content are included at the end of the issue.

Screenshot 2023-03-07 at 3 09 33 PM

Features I'd like to add

  • Incorporate separators to display coauthors in an inline list. I imagine this would work similar to the Post Terms block
  • Add support for appearance tools, especially fonts.
  • More author data:
    • Author archive URL, with an option to make the display name a link.
    • Avatars, preferably as an image block similar to the post feature image block. Also with an option to make it a link.
    • Author bio
  • Subscribe to coauthor data so if the authors are changed in the sidebar, that is reflected in the block content.

Questions about potentially incorporating this into Co-Authors Plus

What is the ideal way to fetch the author data in the block editor?

  • I used apiFetch and /coauthors/v1/authors/:post-id for this version.
    • The data in that endpoint seemed especially suited for the existing plugin sidebar.
    • It includes coauthor emails, and is rightfully restricted to people with the capability to assign authors. This can cause permissions errors for people with other roles.
    • The avatars are URLs. It would be nice to have the attachment id ( when one is used ) for the purpose of using the Image Block.
    • I tried using the existing store for its getAuthors function, but found it had a side-effect where it also setAuthors. When I used getAuthors within a query loop, the post I was editing suddenly had 10 authors.
    • There isn't an endpoint yet where I can fetch an author by id, so when providing context to the display name block, I literally passed the display name. In the example of the Post Title block, the provided context can just be the postId. From there I think a store can be used to avoid repeat API calls.
  • In the past when I needed to display co-author data for posts fetched from the API, I added a profile value to the author taxonomy terms. https://github.com/thoughtis/cata-co-authors-plus/blob/0.6.1/includes/api/class-api.php
    • This is non-standard, but maybe we could standardize it.
    • It works in view contexts and would allow us to use the author taxonomy separate from the assigning of co-authors.
    • We could pass the author taxonomy term id as the context and existing WP stores could be used for fetching entity records.

Examples continued from above

The stored content:

<!-- wp:paragraph -->
<p><strong>Example displaying authors of the current post:</strong></p>
<!-- /wp:paragraph -->

<!-- wp:cap/coauthors -->
<!-- wp:cap/coauthor-display-name /-->
<!-- /wp:cap/coauthors -->

<!-- wp:paragraph -->
<p><strong>Example displaying authors in a query loop:</strong></p>
<!-- /wp:paragraph -->

<!-- wp:query {"queryId":0,"query":{"perPage":"2","pages":0,"offset":0,"postType":"post","order":"desc","orderBy":"date","author":"","search":"","exclude":[],"sticky":"","inherit":false}} -->
<div class="wp-block-query">
<!-- wp:post-template -->
<!-- wp:post-title /-->

<!-- wp:cap/coauthors -->
<!-- wp:group {"style":{"spacing":{"blockGap":"var:preset|spacing|20"}},"layout":{"type":"flex","flexWrap":"nowrap"}} -->
<div class="wp-block-group">
<!-- wp:paragraph -->
<p><em>By</em></p>
<!-- /wp:paragraph -->

<!-- wp:cap/coauthor-display-name /-->
</div>
<!-- /wp:group -->
<!-- /wp:cap/coauthors -->
<!-- /wp:post-template -->
</div>
<!-- /wp:query -->

The rendered content:

<p><strong>Example displaying authors of the current post:</strong></p>
<div class="wp-block-cap-coauthors">
	<div class="wp-block-cap-coauthor">
		<p>John Barleycorn</p>
	</div>
	<div class="wp-block-cap-coauthor">
		<p>Kevin McCallister</p>
	</div>
</div>
<p><strong>Example displaying authors in a query loop:</strong></p>
<div class="wp-block-query is-layout-flow">
	<ul class="wp-block-post-template is-layout-flow">
		<li class="wp-block-post post-2422 post type-post status-publish format-standard hentry category-uncategorized">
			<h2 class="wp-block-post-title">Query Loop</h2>
		</li>
		<li class="wp-block-post post-2843 post type-post status-publish format-standard hentry category-uncategorized tag-coauthors">
			<h2 class="wp-block-post-title">Cuius acerbitati uxor grave accesserat incentivum</h2>
			<div class="wp-block-cap-coauthors">
				<div class="wp-block-cap-coauthor">
					<div class="wp-block-group is-nowrap is-layout-flex wp-container-3">
						<p><em>By</em></p>
						<p>Kevin McCallister</p>
					</div>
				</div>
			</div>
		</li>
		<li class="wp-block-post post-2841 post type-post status-publish format-standard hentry category-uncategorized tag-coauthors">
			<h2 class="wp-block-post-title">Post emensos insuperabilis expeditionis eventus</h2>
			<div class="wp-block-cap-coauthors">
				<div class="wp-block-cap-coauthor">
					<div class="wp-block-group is-nowrap is-layout-flex wp-container-5">
						<p><em>By</em></p>
						<p>Test Guest Author</p>
					</div>
				</div>
			</div>
		</li>
	</ul>
</div>
@alecgeatches
Copy link
Contributor

Hello Doug! Thank you for your contributions in this issue. There are a lot of great ideas here. You've identified some valuable missing features (like block support) for the plugin that we'd be happy to have. We've looked over your PR changes and would be happy to support you in getting these merged into the main plugin. We appreciate the care and time you've put into your changes and design questions.

Design questions

Features I'd like to add

  • Incorporate separators to display coauthors in an inline list. I imagine this would work similar to the Post Terms block
  • Add support for appearance tools, especially fonts.
  • More author data:
    • Author archive URL, with an option to make the display name a link.
    • Avatars, preferably as an image block similar to the post feature image block. Also with an option to make it a link.
    • Author bio
  • Subscribe to coauthor data so if the authors are changed in the sidebar, that is reflected in the block content.

A solid yes for all of these!

What is the ideal way to fetch the author data in the block editor?

After reviewing the code, I have some ideas on how I'd approach the limitations of the current sidebar-focused REST API. Please let me know what you think!

  • I used apiFetch and /coauthors/v1/authors/:post-id for this version.
    • The data in that endpoint seemed especially suited for the existing plugin sidebar.
    • It includes coauthor emails, and is rightfully restricted to people with the capability to assign authors. This can cause permissions errors for people with other roles.

I experimented with a regular author assigned to a post, and saw the permissions errors you mentioned when trying to view the new Co-Authors block. It seems strange that an assigned user is not able to see other assigned users due to the permissions model and data.

I think adding a new author data REST endpoint for block data specifically would work here. This should be limited to users assigned to a specific post. Co-authors on a post should be able to query for other author information for block rendering, but only for the posts that they're assigned. The API can provide author data necessary to render blocks without trying to shoehorn data into the more restrictive sidebar API. Ideally we'd use the same code in REST responses that we use for dynamic block rendering to pull relevant author information.

  • The avatars are URLs. It would be nice to have the attachment id ( when one is used ) for the purpose of using the Image Block.

Good idea! A separate REST endpoint could include this and any other information you need to render blocks.

  • I tried using the existing store for its getAuthors function, but found it had a side-effect where it also setAuthors. When I used getAuthors within a query loop, the post I was editing suddenly had 10 authors.
  • There isn't an endpoint yet where I can fetch an author by id, so when providing context to the display name block, I literally passed the display name. In the example of the Post Title block, the provided context can just be the postId. From there I think a store can be used to avoid repeat API calls.

The getAuthors() side-effect is strange but seems fixable from a quick look. Because we only call getAuthors() in a couple of places, we can change the calling code to trigger setAuthors() after existing getAuthors() calls and remove the nested call.

However, if we use a separate REST endpoint + store as pitched above this fix may not be necessary. With a new store we could potentially subscribe to the existing sidebar updates and use this to auto-update co-author block data without needing to alter the sidebar behavior.

The profile trick is useful, but since it's non-standard I'd prefer to stuff extra data in a separate locked-down endpoint.

Hopefully those can help address your questions. These answers are my personal opinion, so if you have different ideas or pushback, please let us know.


Other questions

I encountered a couple of other questions testing the PR:

  • Blocks have separate package.json that refer to the parent's installed node_modules. Would it be possible to consolidate block building to a single npm run build command under the root package.json?
  • The cap/coauthor-display-name block doesn't appear to show anything when inserted directly into a post. It only renders inside of a query block (like your example), or when used as an inner block of cap/coauthors. This was unexpected behaviour to me. How should that block look when inserted outside of a query or wrapping block context?

Thank you!

@douglas-johnson
Copy link
Contributor Author

Hi @alecgeatches thank you for all of this feedback. I will follow up as soon as I can with an implementation plan for your review. I'd like to address the Other questions section though.

Blocks have separate package.json that refer to the parent's installed node_modules. Would it be possible to consolidate block building to a single npm run build command under the root package.json?

Do you have any guidance or documentation on how to do that? I have tried multiple times and never succeeded.

The cap/coauthor-display-name block doesn't appear to show anything when inserted directly into a post. It only renders inside of a query block (like your example), or when used as an inner block of cap/coauthors. This was unexpected behaviour to me. How should that block look when inserted outside of a query or wrapping block context?

It shouldn't be inserted outside of a query or wrapping block context. It should be updated to be restricted to those contexts, with the possible addition of author archive templates.

@alecgeatches
Copy link
Contributor

Blocks have separate package.json that refer to the parent's installed node_modules. Would it be possible to consolidate block building to a single npm run build command under the root package.json?

Do you have any guidance or documentation on how to do that? I have tried multiple times and never succeeded.

@wordpress/scripts at some point added support for multiple blocks by scanning for block.json. It expects a src/ directory by default, but I was able to build on your fork by changing the root build script with --webpack-src-dir:

"scripts": {
    "build": "wp-scripts build --webpack-src-dir=./blocks",
    // ...
}

This dumps the built blocks into a different build/ directory, so it'll need some tweaking to work correctly with new paths. Hopefully that'll do the trick.

The cap/coauthor-display-name block doesn't appear to show anything when inserted directly into a post. It only renders inside of a query block (like your example), or when used as an inner block of cap/coauthors. This was unexpected behavior to me. How should that block look when inserted outside of a query or wrapping block context?

It shouldn't be inserted outside of a query or wrapping block context. It should be updated to be restricted to those contexts, with the possible addition of author archive templates.

Makes sense to me!

@douglas-johnson
Copy link
Contributor Author

douglas-johnson commented Jun 28, 2023

Thanks @alecgeatches

@wordpress/scripts at some point added support for multiple blocks by scanning for block.json. It expects a src/ directory by default, but I was able to build on your fork by changing the root build script with --webpack-src-dir. This dumps the built blocks into a different build/ directory, so it'll need some tweaking to work correctly with new paths. Hopefully that'll do the trick.

That change in the build folder was part of what I considered unsuccessful last time I tried this. The distributable block.json and scripts are no longer colocated with the PHP for calling register_block_type and handling the render_callback. I suppose that can be handled.

Do these two issues also occur for you? I am reasonably sure I'm running the latest @wordpress/scripts. When I ran the build command as you demonstrated, it also created an incomplete /src folder within the /build folder for each block:

Screenshot 2023-06-28 at 3 44 40 PM

and the block.json in the build folder for each block includes nonexistent CSS files:
Screenshot 2023-06-28 at 3 53 08 PM

@alecgeatches
Copy link
Contributor

That change in the build folder was part of what I considered unsuccessful last time I tried this. The distributable block.json and scripts are no longer colocated with the PHP for calling register_block_type and handling the render_callback. I supposed that can be handled.

That's annoying for the organization, I agree. I imagine most people just deal with it.

and the block.json in the build folder for each block includes nonexistent CSS files:

That's definitely weird, and I'm seeing the same behavior. I think it may have to do with the source layout, as wp-scripts build is expecting a flat block folder with block.json and associated files rather than a tree with build/ and src/.

What do you think about stringing together some build commands instead? This setup seems to work with the existing build structure:

"scripts": {
  "build": "npm run build:plugin && npm run build:block-coauthors && npm run build:block-coauthor-display-name",
  "build:plugin": "wp-scripts build",
  "build:block-coauthors": "wp-scripts build --webpack-src-dir=blocks/coauthors/src/ --output-path=blocks/coauthors/build",
  "build:block-coauthor-display-name": "wp-scripts build --webpack-src-dir=blocks/coauthor-display-name/src/ --output-path=blocks/coauthor-display-name/build",

I'm more interested in the convenience of a single npm run build command than doing everything with wp-scripts defaults, so this is a fine solution for me as well.

@douglas-johnson
Copy link
Contributor Author

@alecgeatches That makes sense to me. I may investigate more to see if there are ways to simplify, but we'll get to a single build command somehow.

Can we discuss workflow?

I imagined I would:

  • Enable issues on my fork.
  • Create issues and a v1 milestone.
  • Make pull requests to the main branch of the fork.
  • Make a pull request to this respoitory when the v1 milestone is satisfied.

Would you be able to provide feedback on those issues and help decide which should apply to the v1 milestone? I expect I'll also ask for reviews on the pull requests.

Does this work or is the an alternative strategy that's more appropriate?

@alecgeatches
Copy link
Contributor

@alecgeatches That makes sense to me. I may investigate more to see if there are ways to simplify, but we'll get to a single build command somehow.

@douglas-johnson Sounds great! I'm sure there's smarter way to shorten or combine those steps.

I imagined I would:

  • Enable issues on my fork.
  • Create issues and a v1 milestone.
  • Make pull requests to the main branch of the fork.
  • Make a pull request to this respoitory when the v1 milestone is satisfied.

Would you be able to provide feedback on those issues and help decide which should apply to the v1 milestone? I expect I'll also ask for reviews on the pull requests.

The downside with this approach is that it would move the conversation and code discussion from this plugin's repository to your fork. It would be great if we can find a workflow that will keep more of that in this repo.

Would this work for you?

  • Create a tracking issue for your changes in this repository. Can have discussion in this issue about planning and milestones.
  • Submit code changes via PRs to this repository. When it's possible to break down tasks into atomic chunks (like adding a REST endpoint by itself), we could review and merge changes as you go for next release.
  • If you want, use your fork's issues internally for tracking your own code changes, and bundle those up when desired into a PR. Hopefully, however, you'd be able to use issues and PRs directly in here for discussion.

@douglas-johnson
Copy link
Contributor Author

Would this work for you?

Sure, I can make that work. I'll close this issue and start a new tracking issue.

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

No branches or pull requests

2 participants