Skip to content
This repository has been archived by the owner on Oct 16, 2024. It is now read-only.

In the editor, show a notice that the theme changed #65

Merged
merged 9 commits into from
Feb 22, 2023

Conversation

kienstra
Copy link
Contributor

@kienstra kienstra commented Feb 21, 2023

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

  1. Create a new post
  2. In another browser tab, change the active theme
  3. Wait 1-2 minutes
  4. Expected: The editor shows a notice:

Screenshot 2023-02-20 at 9 35 26 PM

@kienstra kienstra marked this pull request as ready for review February 21, 2023 03:27

export default function receiveActiveTheme( data: Record< string, unknown > ) {
if ( wasThemeChanged( data, patternManager.activeTheme ) ) {
dispatch( 'core/notices' ).createErrorNotice(
Copy link
Contributor Author

@kienstra kienstra Feb 21, 2023

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().

Copy link
Contributor

@dreamwhisper dreamwhisper left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works as described!
Screen Shot 2023-02-21 at 5 12 36 PM

I have a little hesitation on the overall experience after testing this PR - especially when someone like me does the following.

  1. work on a pattern, maybe forget to save.
  2. switch to another theme to check on something.
  3. switch back to the theme I was working on.
  4. refresh my original pattern and get this:

Screen Shot 2023-02-21 at 5 28 15 PM

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.

@kienstra
Copy link
Contributor Author

Hi @dreamwhisper,
Thanks a lot for testing this, and that's a good point about switching themes.

#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.

@kienstra
Copy link
Contributor Author

kienstra commented Feb 21, 2023

The intent of #62 is to prevent:

  1. Create a new pattern
  2. Save it
  3. Switch themes
  4. Refresh the editor
  5. Save the pattern again
  6. It unexpectedly saves to the old theme

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.

Copy link
Contributor

@dreamwhisper dreamwhisper left a 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.

@kienstra
Copy link
Contributor Author

Great idea.

'Please close this tab' at the beginning is better.

@kienstra
Copy link
Contributor Author

Thanks a lot for your review, Jen!

Definitely, let's discuss the change theme workflow as a group.

@kienstra kienstra merged commit 3842aae into main Feb 22, 2023
@kienstra kienstra deleted the add/heartbeat-theme-check branch February 22, 2023 17:18
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants