Reset state instead of creating duplicate modals

[mattheu]

I have picked up the work started by @rahulsprajapati over here: https://github.com/wp-shortcake/shortcake/pull/680/files. This is a tricky problem, but one that we should definitely try and fix.

I tested his code, but I was still seeing some duplicate markup - as you said when clicking the standard add media button instead.

This fix takes a slightly different approach. Instead it tries to always use the same media modal. If it doesn't exist, it creates it. If it does, it updates the state of the existing one. Additionally I had to do a bit of work to reset the state too, to ensure that when a user clicks a button again, they do not see the old state. I guess we avoided this issue before because it just created a new instance of the modal!

I'd appreciate your thoughts on this.

[goldenapples]

I'll have to test this out more systematically, but from what I see this solves the basic issues I was running up against with this.

I'm not sure I like the fact that if you're editing a shortcode and close out the modal, the next time that you open the media modal - say by the "Add Media" button, you get a modal with the values from the state of the last modal you were editing. That seems like almost never what you'd want.

But I guess this is consistent with core's behavior with the other panels of the media view. And - it looks like one of the fixes you have in this commit prevents the sidebar menu from being hidden in this case, so the modal is still usable in this state.

One bug I still see here: if you use the media modal to edit a shortcode, then go back to edit another shortcode, sometimes the title displayed will be from the first shortcode and not the current shortcode being edited. Haven't tracked down the cause of that yet though...

[mattheu]

Thanks for testing. I'll have a look, as well as fix up the CI failures.

[mattheu]

I originally tried to follow core and maintain the previous state after closing. But since you mentioned it, and also @rahulsprajapati reported this as a bug, I'm not so sure. I think that since we have introduced a custom button for post elements, we should reset the state. I have updated to solve this.

I also updated the toolbar code to make sure it updates the button text when switching from edit to update.

I was unable to reproduce your issue with the modal title.

The CI failures are unrelated to this code - I've opened a PR to resolve them here: https://github.com/wp-shortcake/shortcake/pull/690

[goldenapples]

I agree that this behavior makes a lot more sense intuitively. I think this is a big improvement and almost ready.

Now that we're resetting the media controller state to "insert" on closing the modal, I don't see the problem in the modal title field any more. However, the title in the menu still shows the previous shortcode label. (In this screenshot, I've edited a "Twitter" shortcode, closed the modal, and then reopened it by clicking "Add Post Element". The highlighted item in the media menu should be "Insert Post Element", not "Twitter Details".

[mattheu]

Fixed the menu text issue. Thanks for catching that.

[goldenapples]

This is very smooth. I'm still seeing occasional issues with the title, though. For example, insert a new post element using "Add Post Element", then update an existing post element. The title will say "Insert Post Element" the first time. Close it out and reopen it, and the correct title will be displayed. Any ideas what could be causing that behavior?

[rahulsprajapati]

@mattheu @goldenapples Here is my code for using tiny-mce editor for inner content of shortcode ui textarea. https://gist.github.com/rahulsprajapati/b1363acee3864cf344f4fec10d8cbc18

Here hyperlink functionality was not working properly because of the duplicate markup we are generating Issue #679.

@mattheu manage to solve the duplicate markup issue. But after this, my code is not working when I open the shortcode ui box the second time.

[goldenapples]

@rahulsprajapati Here's a few issues I've found with the code you're using above:

• You're targeting the textarea by ID. I think this causes some problems, since even though the ID is unique at any given time, the view is being dynamically replaced. I was able to get your code working better by targetting textarea[name="inner_content"] instead of textarea#inner_content.
• It looks as though the "render_edit" hook is getting called before the edit shortcode view is fully rendered, and as a result, the function which is supposed to set up the TinyMCE editor on render isn't finding an editor to intiialize. I was able to work around this by wrapping the call to loadEditor in a timeout:

  wp.shortcake.hooks.addAction( 'shortcode-ui.render_edit', function() {
      setTimeout( function() {
          editorLoaded = ShortCodeTinyMceEditor.loadEditor( boxOutEditor );
      })
  } );

I don't have a good answer as to why these issues you're reporting are happening with this branch, but not with #680, but I think the workarounds are reasonable enough that they might not need to be fixed here. Do you agree?

I'm looking at the case where the toolbar label doesn't get reset in between states properly; other than that I think this fix is about ready.

[goldenapples]

:ship: :100:

[mattheu]

Thanks for fixing this @goldenapples !