-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Make SaveButton
extensible
#56807
base: trunk
Are you sure you want to change the base?
Make SaveButton
extensible
#56807
Changes from all commits
4c2ddbe
0c91bb3
b55879b
ba23c32
d709e5c
f01589b
055dcaf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,8 @@ | ||
/** | ||
* WordPress dependencies | ||
*/ | ||
import { useMemo, useCallback } from '@wordpress/element'; | ||
import { applyFilters } from '@wordpress/hooks'; | ||
import { useSelect, useDispatch } from '@wordpress/data'; | ||
import { Button } from '@wordpress/components'; | ||
import { __, sprintf } from '@wordpress/i18n'; | ||
|
@@ -52,6 +54,7 @@ export default function SaveButton( { | |
previewingThemeName: previewingTheme?.name?.rendered, | ||
}; | ||
}, [] ); | ||
|
||
const { setIsSaveViewOpened } = useDispatch( editSiteStore ); | ||
|
||
const activateSaveEnabled = isPreviewingTheme() || isDirty; | ||
|
@@ -78,7 +81,27 @@ export default function SaveButton( { | |
} | ||
return __( 'Save' ); | ||
}; | ||
const label = getLabel(); | ||
|
||
/** | ||
* We focus on adding the customization to the SaveButton's `onClick` and `label` for now. | ||
* We will provide the customization to the other entry points (e.g., SavePanel, SaveHub) in the future if needed. | ||
* @see https://github.com/WordPress/gutenberg/pull/56807 | ||
*/ | ||
const onClick = useCallback( () => { | ||
const callback = applyFilters( 'edit-site.SaveButton.onClick', () => | ||
setIsSaveViewOpened( true ) | ||
); | ||
callback(); | ||
}, [ setIsSaveViewOpened ] ); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we also add the dependencies here. Did you get a warning? See #24914 |
||
const label = useMemo( () => { | ||
return applyFilters( 'edit-site.SaveButton.label', getLabel(), { | ||
isSaving, | ||
disabled, | ||
isPreviewingTheme, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It might be better to add it to the deps as well even if it won't be changed. But the naming is still confusing as people may not notice it's a function instead of the boolean value. However, it's out of the scope of this PR 🙂 |
||
defaultLabel, | ||
isDirty, | ||
} ); | ||
}, [ isSaving, disabled, defaultLabel, isDirty, getLabel ] ); | ||
|
||
return ( | ||
<Button | ||
|
@@ -87,7 +110,7 @@ export default function SaveButton( { | |
aria-disabled={ disabled } | ||
aria-expanded={ isSaveViewOpen } | ||
isBusy={ isSaving } | ||
onClick={ disabled ? undefined : () => setIsSaveViewOpened( true ) } | ||
onClick={ disabled ? undefined : onClick } | ||
label={ label } | ||
/* | ||
* We want the tooltip to show the keyboard shortcut only when the | ||
|
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 think we can just return the function but it's optional 🙂
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 like this idea too.