-
Notifications
You must be signed in to change notification settings - Fork 6
In the editor, show a notice that the theme changed #65
Conversation
|
||
export default function receiveActiveTheme( data: Record< string, unknown > ) { | ||
if ( wasThemeChanged( data, patternManager.activeTheme ) ) { | ||
dispatch( 'core/notices' ).createErrorNotice( |
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.
Instead of stubbing dispatch()
, the unit test only tests wasThemeChanged()
.
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 have a little hesitation on the overall experience after testing this PR - especially when someone like me does the following.
- work on a pattern, maybe forget to save.
- switch to another theme to check on something.
- switch back to the theme I was working on.
- refresh my original pattern and get this:
The pattern really isn't deleted though, so I panic. And, I may have lost some work if I didn't save. Clearly it would be my fault, but I'm used to revisions in WP.
Hi @dreamwhisper, #62 probably caused the 'You attempted to edit an item that doesn't exist.' That's definitely not good to come back to a theme and have it look like the pattern was deleted. |
The intent of #62 is to prevent:
As you noticed, after step 4 (refreshing the editor), it now shows 'You attempted to edit an item that doesn't exist.' That's not great either. So maybe we'll have to figure out which is less of a problem. Or a better way to handle it. |
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'm cool with merging this as is for now. We may want to discuss the experience as a group.
I've been toying with messaging. Does this make it more clear as is?
Please close this tab. This pattern does not exist in the current theme or the theme was changed since this tab was opened.
Great idea. 'Please close this tab' at the beginning is better. |
Thanks a lot for your review, Jen! Definitely, let's discuss the change theme workflow as a group. |
Like Phil suggested, this gets whether the theme changed via the heartbeat API.
If it changed, it shows a notice.
Fixes part of #57 (Send a notice through the heartbeat API to any patterns still open that says The theme was changed. This pattern does not exist in this theme. Please close this tab.)
How to test