-
Notifications
You must be signed in to change notification settings - Fork 51
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
feat: add custom bylines #3667
feat: add custom bylines #3667
Conversation
1f7fe38
to
5094dbd
Compare
7790cc7
to
5a259e9
Compare
src/bylines/index.js
Outdated
const prependBylineToContent = text => { | ||
const contentEl = document.querySelector( '.wp-block-post-content' ); | ||
if ( contentEl ) { | ||
let bylineEl = document.getElementById( BYLINE_ID ); | ||
if ( ! bylineEl ) { | ||
bylineEl = document.createElement( 'div' ); | ||
bylineEl.id = BYLINE_ID; | ||
contentEl.insertBefore( bylineEl, contentEl.firstChild ); | ||
} | ||
if ( isFetching ) { | ||
bylineEl.innerHTML = __( 'Loading…', 'newspack-plugin' ); | ||
return; | ||
} | ||
// If there are author tags | ||
if ( /<Author id=(\d+)>/.test( text ) ) { | ||
console.info( 'Updating byline…' ); // eslint-disable-line no-console | ||
text = text.replace( /<Author id=(\d+)>([^<]+)<\/Author>/g, ( match, authorId, authorName ) => { | ||
const authorData = authors.find( a => a.id === parseInt( authorId ) ); | ||
return authorData ? `<a href="/author/${ authorData.name }">${ authorName }</a>` : authorName; | ||
} ); | ||
} | ||
bylineEl.innerHTML = text; | ||
} | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is another super hacky solution to getting the byline to render in the editor. Ideally we replace this when implementing the full feature.
@leogermani, I've got this PR mostly ready for review assuming its enough to render the byline in the post editor only for the purposes of migration. I didn't want to do too much work on rendering this on the front-end without understanding the requirements fully here. That said, if we'd rather figure this out now, I can get that added in the PR as well. |
757b191
to
6b6de23
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The link was not added for me att first. Then I noticed you were checking to see if the ID exists, but the fetch for authors in the useEffect
only returns 10 authors, so it will never find the ID I was using.
But even when I used one of the returned IDs, the link was broken, with nothing after the /author/
part.
I don't think we need to verify anything at this point. Just create a link. And I think we can go siteurl.com/?author=$ID
, no need to get their slugs.
Finally, let's also add a filter to the_content
and render the byline on the front end as well. This will make QA easier
Thanks for the review @leogermani!
Sure thing! Updated in f8ec6b8
Makes sense. Added in f8ceb44 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added a few more things to the Readme. Thanks for the changes!
Hey @chickenn00dle, good job getting this PR merged! 🎉 Now, the Please check if this PR needs to be included in the "Upcoming Changes" and "Release Notes" doc. If it doesn't, simply remove the label. If it does, please add an entry to our shared document, with screenshots and testing instructions if applicable, then remove the label. Thank you! ❤️ |
# [5.13.0-alpha.1](v5.12.2...v5.13.0-alpha.1) (2025-01-23) ### Bug Fixes * add supported gateways check ([#3650](#3650)) ([74f7773](74f7773)) * **corrections:** replace deprecated sanitize method ([#3694](#3694)) ([ce50e24](ce50e24)) * remove support for legacy form checkout ([#3691](#3691)) ([46a3c16](46a3c16)) * **wcs:** expire manual subscriptions after on-hold duration ([#3681](#3681)) ([658416c](658416c)) ### Features * add custom bylines ([#3667](#3667)) ([3f45a6f](3f45a6f)) * rate limit checkout attempts ([#3678](#3678)) ([d275524](d275524))
🎉 This PR is included in version 5.13.0-alpha.1 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
# [5.13.0](v5.12.5...v5.13.0) (2025-02-03) ### Bug Fixes * add supported gateways check ([#3650](#3650)) ([74f7773](74f7773)) * **corrections:** replace deprecated sanitize method ([#3694](#3694)) ([ce50e24](ce50e24)) * remove support for legacy form checkout ([#3691](#3691)) ([46a3c16](46a3c16)) * **wcs:** expire manual subscriptions after on-hold duration ([#3681](#3681)) ([658416c](658416c)) ### Features * add custom bylines ([#3667](#3667)) ([3f45a6f](3f45a6f)) * rate limit checkout attempts ([#3678](#3678)) ([d275524](d275524)) * **reader-revenue:** add PayPal Payments gateway to wizard ([#3665](#3665)) ([1476eed](1476eed))
🎉 This PR is included in version 5.13.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
All Submissions:
Changes proposed in this Pull Request:
Closes https://app.asana.com/0/1208894036683495/1208945901567231 and https://app.asana.com/0/1208894036683495/1208945901567229
This PR adds a barebones custom byline feature to posts behind a
NEWSPACK_BYLINES_ENABLED
feature flag.How to test the changes in this Pull Request:
Newspack Custom Byline
panel is present in the post editor sidebar<Author id=USER_ID>Name</Author>
tag<Invalid>
) and confirm an alert appears warning you onlyAuthor
is supportedOther information: