Skip to content
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

Decide whether Shortcake is coupled with TinyMCE or the media library #153

Closed
danielbachhuber opened this issue Feb 13, 2015 · 16 comments
Closed
Labels
Milestone

Comments

@danielbachhuber
Copy link
Contributor

The media library can be used independently of the editor, and vice versa.

We need more robust code for making sure Shortcake is only included for the editor

From #149

@danielbachhuber
Copy link
Contributor Author

I think Shortcake should be coupled with the editor, and make sure appropriate media assets are enqueued if the media library hasn't already been enqueued.

@danielbachhuber danielbachhuber added this to the next milestone Feb 13, 2015
@danielbachhuber
Copy link
Contributor Author

@sanchothefat Can you look into a better solution for this, given that you were keen on fixing it?

@mattheu
Copy link
Contributor

mattheu commented Feb 13, 2015

At the moment we're pretty coupled to both. The UI is completely tied to the media library, and the rendering is completely coupled to the editor.

However - I think it actually makes more sense to couple with the media library - as you currenlty can't use without.

I can easily see the need to use the Shortcake UI outside of the editor. We pretty much have this requirement on Fusion at the moment.

It would require more significant changes to use the UI outside of the Media Library - however I see it as a definite possibility following #142

@roborourke
Copy link
Contributor

@danielbachhuber we're kind of at the mercy of WordPress core here, the print_media_templates action is coupled to the media library so we'd have to use wp_enqueue_editor and a generic hook like admin_print_footer_scripts instead.

@danielbachhuber
Copy link
Contributor Author

I can easily see the need to use the Shortcake UI outside of the editor. We pretty much have this requirement on Fusion at the moment.

Huh? Where do previews appear, if not within TinyMCE?

@danielbachhuber
Copy link
Contributor Author

FWIW, #149 causes JS errors, so I need to revert.

@danielbachhuber
Copy link
Contributor Author

we're kind of at the mercy of WordPress core here, the print_media_templates action is coupled to the media library so we'd have to use wp_enqueue_editor and a generic hook like admin_print_footer_scripts instead.

That's fine. I think we should ditch print_media_templates as it's been causing us troubles. See #139

@roborourke
Copy link
Contributor

Can you elaborate on the js errors you saw? I was getting the same problem hence the PR

@danielbachhuber
Copy link
Contributor Author

Can you elaborate on the js errors you saw?

An internal JS file that has dependencies on Shortcake started throwing an undefined variable error.

I don't think the solution is to change the load action. I think the solution is to internalize the printing of our own templates, and also calling wp_enqueue_media() to make sure the media library assets already always available.

@mattheu
Copy link
Contributor

mattheu commented Feb 13, 2015

Well currently they're very inter-linked - but I think it should be possible to further decouple the preview rendering and the TinyMCE.

Similar to how you can have a Media field in Fieldmanager - that shows a button that then inserts the image ID into a hidden field. Fieldmanager then renders the image preview.

You could have a similar button to trigger the shortcake UI, and inserts the shortcode into a hidden field. You could then use the existing do_shortcode ajax action to get a preview. I feel like we're not far off being able to do this.

Of course you would have to handle this all yourself. In which case - you could probably also handle loading the correct templates...

@bfintal
Copy link
Contributor

bfintal commented Feb 13, 2015

In my opinion, behavior-wise, couple it with Media Library because the UI is there. The "Add Media" (or hopefully the "Add Shortcode" button or something else to be placed beside the current button) would tie the output towards TinyMCE. Then the preview can be there regardless of TinyMCE being there or not.

@roborourke
Copy link
Contributor

So we need a public enqueue_shortcode_ui() function?

@danielbachhuber
Copy link
Contributor Author

You could have a similar button to trigger the shortcake UI, and inserts the shortcode into a hidden field.

Oh, I see where you're going. Not sure why you'd use a shortcode to store data in this case, but I can see the case for being able to reuse the modal UI.

In my opinion, behavior-wise, couple it with Media Library because the UI is there.

It depends on how you look at it. I see it as the UI being in TinyMCE with Media Library as an artifact of the implementation decision. If we build #116 (inline editing), we're no longer coupled with the media library.

Regardless, it sounds like we need to decouple into two parts: the media library component and the TinyMCE component. This should probably inform #142

@bfintal
Copy link
Contributor

bfintal commented Feb 13, 2015

If we build #116 (inline editing), we're no longer coupled with the media library.

#116 can be an entirely different thing if only the editable parts would be the ones that can be seen in the rendered view.

You can be totally independent from the Media Library if all the settings are somehow editable in TinyMCE though.

@mattheu
Copy link
Contributor

mattheu commented Feb 13, 2015

I think the solution is to internalize the printing of our own templates, and also calling wp_enqueue_media() to make sure the media library assets already always available.

Agreed on this.

Regarding #116 - I think it is further argument that decoupling all aspects from each other is a good move. Then you can use the bits you need from shortcake in the places you need them.

@danielbachhuber
Copy link
Contributor Author

Our enqueue_shortcode_ui action has proved sufficient for loosely-coupling assets. We'll continue with the conceptual paradigm discussion in #281

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants