-
Notifications
You must be signed in to change notification settings - Fork 4.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
Add indicator for metadata changes to Save Panel when reviewing modified entities #61811
Changes from 14 commits
941fd19
a13c172
58c06bd
6d1c37b
3582092
439c0da
97bdf0e
dbd7989
059b7d4
c0217d1
63cef47
ce4ade3
cfd0f26
804761c
83abcd7
8f1df20
f681800
baf5ff7
cd0ceeb
4172d9f
c1a756a
23d9429
2b509ce
2d8fb99
1511a08
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,49 +1,78 @@ | ||
/** | ||
* WordPress dependencies | ||
*/ | ||
import { CheckboxControl, PanelRow } from '@wordpress/components'; | ||
import { Icon, CheckboxControl, Flex, PanelRow } from '@wordpress/components'; | ||
import { __ } from '@wordpress/i18n'; | ||
import { useSelect } from '@wordpress/data'; | ||
import { store as coreStore } from '@wordpress/core-data'; | ||
import { decodeEntities } from '@wordpress/html-entities'; | ||
import { connection } from '@wordpress/icons'; | ||
|
||
/** | ||
* Internal dependencies | ||
*/ | ||
import { store as editorStore } from '../../store'; | ||
import { unlock } from '../../lock-unlock'; | ||
|
||
export default function EntityRecordItem( { record, checked, onChange } ) { | ||
const { name, kind, title, key } = record; | ||
|
||
// Handle templates that might use default descriptive titles. | ||
const entityRecordTitle = useSelect( | ||
const { entityRecordTitle, hasPostMetaChanges } = useSelect( | ||
( select ) => { | ||
if ( 'postType' !== kind || 'wp_template' !== name ) { | ||
return title; | ||
return { | ||
entityRecordTitle: title, | ||
hasPostMetaChanges: unlock( | ||
select( editorStore ) | ||
).hasPostMetaChanges(), | ||
}; | ||
} | ||
|
||
const template = select( coreStore ).getEditedEntityRecord( | ||
kind, | ||
name, | ||
key | ||
); | ||
return select( editorStore ).__experimentalGetTemplateInfo( | ||
template | ||
).title; | ||
return { | ||
entityRecordTitle: | ||
select( editorStore ).__experimentalGetTemplateInfo( | ||
template | ||
).title, | ||
hasPostMetaChanges: unlock( | ||
select( editorStore ) | ||
).hasPostMetaChanges(), | ||
}; | ||
}, | ||
[ name, kind, title, key ] | ||
); | ||
|
||
return ( | ||
<PanelRow> | ||
<CheckboxControl | ||
__nextHasNoMarginBottom | ||
label={ | ||
decodeEntities( entityRecordTitle ) || __( 'Untitled' ) | ||
} | ||
checked={ checked } | ||
onChange={ onChange } | ||
/> | ||
</PanelRow> | ||
<> | ||
<PanelRow> | ||
<CheckboxControl | ||
__nextHasNoMarginBottom | ||
label={ | ||
decodeEntities( entityRecordTitle ) || __( 'Untitled' ) | ||
} | ||
checked={ checked } | ||
onChange={ onChange } | ||
/> | ||
</PanelRow> | ||
{ hasPostMetaChanges && ( | ||
<PanelRow> | ||
<Flex className="entities-saved-states__block-bindings"> | ||
<Icon | ||
className="entities-saved-states__connections-icon" | ||
icon={ connection } | ||
size={ 24 } | ||
/> | ||
<span className="entities-saved-states__bindings-text"> | ||
{ __( 'Post Meta.' ) } | ||
</span> | ||
</Flex> | ||
</PanelRow> | ||
) } | ||
</> | ||
); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,6 +14,7 @@ import PostPublishPanel from '../post-publish-panel'; | |
import PluginPrePublishPanel from '../plugin-pre-publish-panel'; | ||
import PluginPostPublishPanel from '../plugin-post-publish-panel'; | ||
import { store as editorStore } from '../../store'; | ||
import { unlock } from '../../lock-unlock'; | ||
|
||
const { Fill, Slot } = createSlotFill( 'ActionsPanel' ); | ||
|
||
|
@@ -27,12 +28,19 @@ export default function SavePublishPanels( { | |
} ) { | ||
const { closePublishSidebar, togglePublishSidebar } = | ||
useDispatch( editorStore ); | ||
const { publishSidebarOpened, hasNonPostEntityChanges } = useSelect( | ||
const { | ||
publishSidebarOpened, | ||
hasNonPostEntityChanges, | ||
hasPostMetaChanges, | ||
} = useSelect( | ||
( select ) => ( { | ||
publishSidebarOpened: | ||
select( editorStore ).isPublishSidebarOpened(), | ||
hasNonPostEntityChanges: | ||
select( editorStore ).hasNonPostEntityChanges(), | ||
hasPostMetaChanges: unlock( | ||
select( editorStore ) | ||
).hasPostMetaChanges(), | ||
} ), | ||
[] | ||
); | ||
|
@@ -54,7 +62,7 @@ export default function SavePublishPanels( { | |
PostPublishExtension={ PluginPostPublishPanel.Slot } | ||
/> | ||
); | ||
} else if ( hasNonPostEntityChanges ) { | ||
} else if ( hasNonPostEntityChanges || hasPostMetaChanges ) { | ||
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. To be honest, I don't fully understand what this part of the code does and if it is necessary or not. 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. As far as I remember, this alternative flow triggers when you edit the template of the post. Let me record a screencast to better illustrate it. 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. save-flow.movThere 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. In effect, what I think we should achieve with this PR would be:
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. If I am not mistaken, that is caused because the selector gets the type and id using 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. In my opinion, it should use the selector from 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. Do you mean using 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. I pushed this commit to illustrate what I mean, but I am happy to iterate once I understand better your suggestion. If I am not mistaken, that change fixes the problems you mention. 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. I tested @SantosGuillamot's fix for to address the scenario wherein the indicator for post meta changes didn't appear when modifying posts in the query loop. It appears to work: |
||
unmountableContent = ( | ||
<div className="editor-layout__toggle-entities-saved-states-panel"> | ||
<Button | ||
|
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 assume we should get
hasPostMetaChanges
from here and not in the component props, right?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 just changed it. Feel free to revert it if you consider it necessary.