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

Followers Block #344

Merged
merged 49 commits into from Jul 26, 2023
Merged

Followers Block #344

merged 49 commits into from Jul 26, 2023

Conversation

mattwiebe
Copy link
Contributor

@mattwiebe mattwiebe commented Jun 2, 2023

This adds a Follower block so that sites can display their fediverse followers on the frontend!

Needs testing instructions, but this is mostly done.

Proposed changes:

Other information:

  • Have you written new tests for your changes, if applicable?

Testing instructions:

  • Go to '..'

@mattwiebe
Copy link
Contributor Author

mattwiebe commented Jun 2, 2023

I don't love adding /build but it's useful for now.

(I also don't love plugins that need to be built before they can be used and I greatly appreciated that this was not one of them and now here I am potentially ruining it!)

@mattwiebe mattwiebe requested a review from pfefferle June 2, 2023 19:31
@mediaformat
Copy link
Collaborator

mediaformat commented Jun 5, 2023

I don't love adding /build but it's useful for now.

@mattwiebe I might need to use it for another thing, perhaps this block code could go under src/followers or something

src/index.js Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@mattwiebe
Copy link
Contributor Author

I might need to use it for another thing, perhaps this block code could go under src/followers or something

@mediaformat That should be easy enough. We'll have to decide if we want to have built artifacts checked in or not.

includes/model/class-follower.php Outdated Show resolved Hide resolved
includes/model/class-follower.php Outdated Show resolved Hide resolved
includes/model/class-follower.php Outdated Show resolved Hide resolved
includes/model/class-follower.php Outdated Show resolved Hide resolved
@mattwiebe mattwiebe changed the title Follower Block Followers Block Jul 25, 2023
Comment on lines 47 to 50
__( '←', 'activitypub' )
}
</span>
{ __( 'Less', 'activitypub' ) }
Copy link
Member

Choose a reason for hiding this comment

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

Maybe this (and the below) could be combined into a single translatable string, with createInterpolateElement from @wordpress/element. It would make things easier in RTL languages, for example.

Something like this (not tested):

diff --git a/src/followers/followers.js b/src/followers/followers.js
index aee1c3c..b756f8f 100644
--- a/src/followers/followers.js
+++ b/src/followers/followers.js
@@ -1,6 +1,7 @@
 import { useState, useEffect } from 'react';
 import apiFetch from '@wordpress/api-fetch';
 import { addQueryArgs } from '@wordpress/url';
+import { createInterpolateElement } from '@wordpress/element';
 import { __ } from '@wordpress/i18n';
 import { Pagination } from './pagination';
 import { ExternalLink } from '@wordpress/components';
@@ -39,27 +40,19 @@ export function Followers( {
 	const [ localPage, setLocalPage ] = usePage();
 	const page = passedPage || localPage;
 	const setPage = passedSetPage || setLocalPage;
-	const prevLabel = (
-		<>
-			<span class="wp-block-query-pagination-previous-arrow is-arrow-arrow" aria-hidden="true">
-				{
-					/* translators: arrow for previous followers link */
-					__( '←', 'activitypub' )
-				}
-			</span>
-			{ __( 'Less', 'activitypub' ) }
-		</>
+	const prevLabel = createInterpolateElement(
+		/* translators: arrow for previous followers link */
+		__( '<span>←</span> Less', 'activitypub' ),
+		{
+			span: <span class="wp-block-query-pagination-previous-arrow is-arrow-arrow" aria-hidden="true" />,
+		}
 	);
-	const nextLabel = (
-		<>
-			{ __( 'More', 'activitypub' ) }
-			<span class="wp-block-query-pagination-next-arrow is-arrow-arrow" aria-hidden="true">
-				{
-					/* translators: arrow for next followers link */
-					__( '→', 'activitypub' )
-				}
-			</span>
-		</>
+	const nextLabel = createInterpolateElement(
+		/* translators: arrow for next followers link */
+		__( 'More <span>→</span>', 'activitypub' ),
+		{
+			span: <span class="wp-block-query-pagination-next-arrow is-arrow-arrow" aria-hidden="true" />,
+		}
 	);
 
 	useEffect( () => {
@@ -109,4 +102,4 @@ function Follower( { name, icon, url, preferredUsername } ) {
 			</span>
 		</ExternalLink>
 	)
-}
\ No newline at end of file
+}

Copy link
Contributor Author

@mattwiebe mattwiebe Jul 26, 2023

Choose a reason for hiding this comment

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

TIL about createInterpolateElement - thanks so much for this, great point about RTL and just keeping the context together. dd0f39a

pfefferle
pfefferle previously approved these changes Jul 26, 2023
Copy link
Member

@pfefferle pfefferle left a comment

Choose a reason for hiding this comment

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

oh, you are right!

Copy link
Member

@pfefferle pfefferle left a comment

Choose a reason for hiding this comment

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

😍

@mattwiebe mattwiebe merged commit 5b9dadd into master Jul 26, 2023
18 checks passed
@pfefferle pfefferle deleted the add/follower-block branch July 27, 2023 08:33
pfefferle added a commit that referenced this pull request Aug 10, 2023
Introduces a new Followers block. Proudly display your Fediverse followers to the world!

---------

Co-authored-by: Matthias Pfefferle <pfefferle@users.noreply.github.com>
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.

None yet

4 participants