-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Donation block: Make content editable #43834
Conversation
This PR does not affect the size of JS and CSS bundles shipped to the user's browser. Generated by performance advisor bot at iscalypsofastyet.com. |
@gwwar you're welcome to take this over if you want, otherwise I'll continue with it tomorrow. Most of the issues I'm dealing with are caused by a fragile approach implementing the tab navigation (only one tab should be visible at time, but content of non-visible tabs should be preserved), but I guess there might be blocks that should have solved this before (maybe the slideshow block?). |
@@ -72,8 +72,15 @@ const Edit = ( { attributes, setAttributes } ) => { | |||
}; | |||
|
|||
useEffect( () => { | |||
const updateData = () => fetchStatus( 'donation' ).then( mapStatusToState, apiError ); | |||
updateData(); | |||
const getStatus = async () => { |
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 can be reverted. I changed this to debug easier a failure on my dev, but forgot to leave as it was before.
}; | ||
|
||
const buttons = { | ||
'one-time': __( 'Donate', 'full-site-editing' ), |
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.
ℹ️ String reuse speeds up translation and improves consistency. The following string might make a good alternative and has already been translated 79 times:
translate( 'Donate' )
ES Score: 13
className={ classNames( { 'is-active': isTabActive( 'one-time' ) } ) } | ||
onClick={ () => setActiveTab( 'one-time' ) } | ||
> | ||
{ __( 'One-Time', 'full-site-editing' ) } |
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.
ℹ️ String reuse speeds up translation and improves consistency. The following string might make a good alternative and has already been translated 3 times:
translate( 'one-time' )
ES Score: 7
className={ classNames( { 'is-active': isTabActive( '1 month' ) } ) } | ||
onClick={ () => setActiveTab( '1 month' ) } | ||
> | ||
{ __( 'Monthly', 'full-site-editing' ) } |
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.
ℹ️ String reuse speeds up translation and improves consistency. The following string might make a good alternative and has already been translated 69 times:
translate( 'Monthly' )
ES Score: 13
See 1 additional suggestions in the PR translation status page
className={ classNames( { 'is-active': isTabActive( '1 year' ) } ) } | ||
onClick={ () => setActiveTab( '1 year' ) } | ||
> | ||
{ __( 'Annually', 'full-site-editing' ) } |
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.
ℹ️ String reuse speeds up translation and improves consistency. The following string might make a good alternative and has already been translated 18 times:
translate( 'billed annually' )
ES Score: 7
See 1 additional suggestions in the PR translation status page
</div> | ||
</div> | ||
<InspectorControls> | ||
<PanelBody title={ __( 'Settings', 'full-site-editing' ) }> |
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.
ℹ️ String reuse speeds up translation and improves consistency. The following string might make a good alternative and has already been translated 42 times:
translate( 'Settings', { context: 'NewDash Page Title'} )
ES Score: 8
See 2 additional suggestions in the PR translation status page
Caution: This PR affects files in the FSE Plugin on WordPress.com D45779-code has been created so you can easily test it on your sandbox. See this FieldGuide page about developing in the FSE Plugin for more info: PCYsg-ly5-p2 |
Could solve them in aec78f8 by replicating what we did in the premium content block: create a context so the active tab is shared across the nested blocks, allowing them to toggle their visibility. |
const { showMonthly, showAnnually, showCustom } = attributes; | ||
return ( | ||
<InspectorControls> | ||
<PanelBody title={ __( 'Settings', 'full-site-editing' ) }> |
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.
ℹ️ String reuse speeds up translation and improves consistency. The following string might make a good alternative and has already been translated 42 times:
translate( 'Settings', { context: 'NewDash Page Title'} )
ES Score: 8
See 2 additional suggestions in the PR translation status page
…rather than multiple nested blocks
const { showMonthly, showAnnual, showCustom } = attributes; | ||
return ( | ||
<InspectorControls> | ||
<PanelBody title={ __( 'Settings', 'full-site-editing' ) }> |
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.
ℹ️ String reuse speeds up translation and improves consistency. The following string might make a good alternative and has already been translated 42 times:
translate( 'Settings', { context: 'NewDash Page Title'} )
ES Score: 8
See 2 additional suggestions in the PR translation status page
}, | ||
oneTimeButtonText: { | ||
type: 'string', | ||
default: __( 'Donate', 'full-site-editing' ), |
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.
ℹ️ String reuse speeds up translation and improves consistency. The following string might make a good alternative and has already been translated 79 times:
translate( 'Donate' )
ES Score: 13
const isTabActive = ( tab ) => activeTab === tab; | ||
|
||
const tabs = { | ||
'one-time': { title: __( 'One-Time', 'full-site-editing' ) }, |
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.
ℹ️ String reuse speeds up translation and improves consistency. The following string might make a good alternative and has already been translated 3 times:
translate( 'one-time' )
ES Score: 7
|
||
const tabs = { | ||
'one-time': { title: __( 'One-Time', 'full-site-editing' ) }, | ||
...( showMonthly && { '1 month': { title: __( 'Monthly', 'full-site-editing' ) } } ), |
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.
ℹ️ String reuse speeds up translation and improves consistency. The following string might make a good alternative and has already been translated 69 times:
translate( 'Monthly' )
ES Score: 13
See 1 additional suggestions in the PR translation status page
const tabs = { | ||
'one-time': { title: __( 'One-Time', 'full-site-editing' ) }, | ||
...( showMonthly && { '1 month': { title: __( 'Monthly', 'full-site-editing' ) } } ), | ||
...( showAnnual && { '1 year': { title: __( 'Yearly', 'full-site-editing' ) } } ), |
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.
ℹ️ String reuse speeds up translation and improves consistency. The following string might make a good alternative and has already been translated 58 times:
translate( 'Yearly' )
ES Score: 15
See 1 additional suggestions in the PR translation status page
|
||
const tabs = { | ||
'one-time': { title: __( 'One-Time', 'full-site-editing' ) }, | ||
...( monthlyPlanId && { '1 month': { title: __( 'Monthly', 'full-site-editing' ) } } ), |
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.
ℹ️ String reuse speeds up translation and improves consistency. The following string might make a good alternative and has already been translated 69 times:
translate( 'Monthly' )
ES Score: 13
See 1 additional suggestions in the PR translation status page
const tabs = { | ||
'one-time': { title: __( 'One-Time', 'full-site-editing' ) }, | ||
...( monthlyPlanId && { '1 month': { title: __( 'Monthly', 'full-site-editing' ) } } ), | ||
...( annuallyPlanId && { '1 year': { title: __( 'Yearly', 'full-site-editing' ) } } ), |
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.
ℹ️ String reuse speeds up translation and improves consistency. The following string might make a good alternative and has already been translated 58 times:
translate( 'Yearly' )
ES Score: 15
See 1 additional suggestions in the PR translation status page
Yes, we'll want to store the tier amounts within the block, and have this be editable. This only makes sense for this block since this is technically backed by a pay-what-you-want plan. |
Nice work @mmtr ! 💖 Looks like we're maybe aiming for a dynamic block format to start with? This does give us a bit more flexibility especially since block updates still aren't the smoothest for folks. I'll go ahead and land this, and will continue work next week. |
I also double checked the phpcs failures related to donation locally, and it seems like it's complaining about the asset files which we can ignore. |
This Pull Request is now available for translation here: https://translate.wordpress.com/deliverables/4124308 Thank you @mmtr for including a screenshot in the description! This is really helpful for our translators. |
Translation for this Pull Request has now been finished. |
Changes proposed in this Pull Request
Adds the ability to edit the default content included in a donations block.
Testing instructions
cd apps/full-site-editing; yarn dev --sync
.fse-donations-block
blog sticker applied (if not, it can be done from the site's Blog RC).To follow up
Items below are out of scope of this PR and will be implemented in follow-ups.
Part of #42856