-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Add Reusable block save button, snackbar on save and Welcome Guide #32464
Changes from 21 commits
8bc50d4
fd391bd
afe9922
424fbc9
a4f9b21
297e62e
ea6ef3a
38fbea7
355595a
1046bc4
35e713b
a0ab538
da40570
1952b92
cd7bca8
0df63a4
2a213ee
74fcdd6
f038b11
6e5b545
bf3eaf9
740a109
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 |
---|---|---|
@@ -0,0 +1,42 @@ | ||
/** | ||
* WordPress dependencies | ||
*/ | ||
import { Fill } from '@wordpress/components'; | ||
import { useEffect } from '@wordpress/element'; | ||
|
||
function UnsavedChangesIndicator() { | ||
useEffect( () => { | ||
// add class to the parent selector to increase it's width | ||
const parentSelectorBtn = document.querySelector( | ||
'.block-editor-block-parent-selector' | ||
); | ||
parentSelectorBtn.classList.add( 'parent-block-has-changes' ); | ||
// add class to the contextual toolbar to move it to the right side | ||
const contextualToolBar = document.querySelector( | ||
'.block-editor-block-contextual-toolbar' | ||
); | ||
contextualToolBar.classList.add( 'parent-block-has-changes' ); | ||
|
||
return () => { | ||
// remove classes from the parent selector and contextual toolbar when component unmounts | ||
parentSelectorBtn.classList.remove( 'parent-block-has-changes' ); | ||
contextualToolBar.classList.remove( 'parent-block-has-changes' ); | ||
}; | ||
Comment on lines
+9
to
+24
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. Something that I mentioned on the other PR (thisissandip#1), I think it'd be good to make this slot not modify the element's class names in this way. This couples the I recall it was because the parent selector button is positioned absolutely. That seems unusual to me, and like some tech debt. Maybe those toolbar styles can be refactored. |
||
}, [] ); | ||
|
||
return ( | ||
<div className="parent-block-selector-unsaved-changes-indicator"></div> | ||
); | ||
} | ||
|
||
function ParentBlockSelectorUnsavedChangesIndicator() { | ||
return ( | ||
<> | ||
<Fill name="parent-block-selector-unsaved-changes-indicator"> | ||
<UnsavedChangesIndicator /> | ||
</Fill> | ||
</> | ||
); | ||
} | ||
|
||
export default ParentBlockSelectorUnsavedChangesIndicator; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
/** | ||
* WordPress dependencies | ||
*/ | ||
import { Fill } from '@wordpress/components'; | ||
|
||
function UnsavedChangesIndicator() { | ||
return <div className="block-unsaved-changes-indicator"></div>; | ||
} | ||
|
||
function SelectedBlockUnsavedChangesIndicator() { | ||
return ( | ||
<> | ||
<Fill name="block-unsaved-changes-indicator"> | ||
<UnsavedChangesIndicator /> | ||
</Fill> | ||
</> | ||
); | ||
} | ||
|
||
export default SelectedBlockUnsavedChangesIndicator; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,25 +15,35 @@ import { | |
TextControl, | ||
PanelBody, | ||
} from '@wordpress/components'; | ||
import { useState } from '@wordpress/element'; | ||
import { __ } from '@wordpress/i18n'; | ||
import { | ||
__experimentalUseInnerBlocksProps as useInnerBlocksProps, | ||
__experimentalUseNoRecursiveRenders as useNoRecursiveRenders, | ||
__experimentalBlockContentOverlay as BlockContentOverlay, | ||
__experimentalParentBlockSelectorUnsavedChangesIndicator as ParentBlockSelectorUnsavedChangesIndicator, | ||
__experimentalSelectedBlockUnsavedChangesIndicator as SelectedBlockUnsavedChangesIndicator, | ||
InnerBlocks, | ||
BlockControls, | ||
InspectorControls, | ||
useBlockProps, | ||
Warning, | ||
store as blockEditorStore, | ||
} from '@wordpress/block-editor'; | ||
import { store as reusableBlocksStore } from '@wordpress/reusable-blocks'; | ||
import { ungroup } from '@wordpress/icons'; | ||
import { store as noticesStore } from '@wordpress/notices'; | ||
|
||
/** | ||
* Internal dependencies | ||
*/ | ||
import ReusableBlockWelcomeGuide from './reusable-block-welcome-guide'; | ||
|
||
export default function ReusableBlockEdit( { attributes: { ref }, clientId } ) { | ||
const [ hasAlreadyRendered, RecursionProvider ] = useNoRecursiveRenders( | ||
ref | ||
); | ||
const { isMissing, hasResolved } = useSelect( | ||
const { isMissing, hasResolved, hasEdits } = useSelect( | ||
( select ) => { | ||
const persistedBlock = select( coreStore ).getEntityRecord( | ||
'postType', | ||
|
@@ -47,14 +57,37 @@ export default function ReusableBlockEdit( { attributes: { ref }, clientId } ) { | |
'wp_block', | ||
ref, | ||
] ); | ||
const blockHasEdits = select( coreStore ).hasEditsForEntityRecord( | ||
'postType', | ||
'wp_block', | ||
ref | ||
); | ||
return { | ||
hasResolved: hasResolvedBlock, | ||
isMissing: hasResolvedBlock && ! persistedBlock, | ||
hasEdits: blockHasEdits, | ||
}; | ||
}, | ||
[ ref, clientId ] | ||
); | ||
|
||
const { isChildSelected, isSelected } = useSelect( ( select ) => { | ||
const { getBlockParents, getSelectedBlockClientId } = select( | ||
blockEditorStore | ||
); | ||
const selectedBlockClientId = getSelectedBlockClientId(); | ||
const _isSelected = selectedBlockClientId === clientId; | ||
|
||
const parents = getBlockParents( selectedBlockClientId ); | ||
const firstParentClientId = parents[ parents.length - 1 ]; | ||
const _isChildSelected = firstParentClientId === clientId; | ||
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. Shouldn't the case of changing nested blocks inside the 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.
Yes, this is much cleaner and better. Thank you for the suggestion. Updating this in my next commit.
Even I am not sure about this case cause one has to select the Reusable block itself to access the save button in the block toolbar. 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.
Introducing dot indicators into List view has come up as one possible solution that would help with more visibility in instances with a lot of nesting. Needs a design explore but I could look at this alongside other follow ups in #33988! |
||
|
||
return { | ||
isChildSelected: _isChildSelected, | ||
isSelected: _isSelected, | ||
}; | ||
}, [] ); | ||
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.
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. Thank you! You saved me from future errors! 🚀 |
||
|
||
const { | ||
__experimentalConvertBlockToStatic: convertBlockToStatic, | ||
} = useDispatch( reusableBlocksStore ); | ||
|
@@ -85,6 +118,31 @@ export default function ReusableBlockEdit( { attributes: { ref }, clientId } ) { | |
} | ||
); | ||
|
||
// local states for gudie modal | ||
const [ isGuideOpen, setIsGuideOpen ] = useState( false ); | ||
|
||
const { saveEditedEntityRecord } = useDispatch( coreStore ); | ||
const { createNotice } = useDispatch( noticesStore ); | ||
|
||
// save the unsaved records | ||
const saveEditedRecords = () => { | ||
saveEditedEntityRecord( 'postType', 'wp_block', ref ); | ||
showSnackbar(); | ||
}; | ||
|
||
const showSnackbar = () => { | ||
createNotice( 'success', __( 'Reusable block saved.' ), { | ||
type: 'snackbar', | ||
isDismissible: true, | ||
actions: [ | ||
{ | ||
label: __( 'Learn more' ), | ||
onClick: () => setIsGuideOpen( true ), | ||
}, | ||
], | ||
} ); | ||
}; | ||
|
||
if ( hasAlreadyRendered ) { | ||
return ( | ||
<div { ...blockProps }> | ||
|
@@ -118,6 +176,14 @@ export default function ReusableBlockEdit( { attributes: { ref }, clientId } ) { | |
return ( | ||
<RecursionProvider> | ||
<div { ...blockProps }> | ||
{ isSelected && hasEdits && ( | ||
<SelectedBlockUnsavedChangesIndicator /> | ||
) } | ||
|
||
{ isChildSelected && hasEdits && ( | ||
<ParentBlockSelectorUnsavedChangesIndicator /> | ||
) } | ||
|
||
<BlockControls> | ||
<ToolbarGroup> | ||
<ToolbarButton | ||
|
@@ -127,6 +193,19 @@ export default function ReusableBlockEdit( { attributes: { ref }, clientId } ) { | |
showTooltip | ||
/> | ||
</ToolbarGroup> | ||
{ hasEdits && ( | ||
<ToolbarGroup> | ||
<ToolbarButton | ||
isPrimary | ||
className="block-library-block__reusable-block-save-button" | ||
onClick={ saveEditedRecords } | ||
label={ __( 'Save globally' ) } | ||
showTooltip | ||
> | ||
{ __( 'Save' ) } | ||
</ToolbarButton> | ||
</ToolbarGroup> | ||
) } | ||
</BlockControls> | ||
<InspectorControls> | ||
<PanelBody> | ||
|
@@ -142,6 +221,12 @@ export default function ReusableBlockEdit( { attributes: { ref }, clientId } ) { | |
wrapperProps={ innerBlocksProps } | ||
className="block-library-block__reusable-block-container" | ||
/> | ||
{ isGuideOpen && ( | ||
<ReusableBlockWelcomeGuide | ||
isGuideOpen={ isGuideOpen } | ||
setIsGuideOpen={ setIsGuideOpen } | ||
/> | ||
) } | ||
</div> | ||
</RecursionProvider> | ||
); | ||
|
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.
Changing this value to -16px (or
$grid-unit-20
) seems to get rid of the visual jump for me (tested on a couple of different screens at different viewport sizes).