embed tab menu needs rewrite

[cash]

Needs to do a better job using the menu system. Also tabs are not being highlighted.

[cash]

I have a version working for this. Would be great to pull it in for 1.8.1 beta to get beta testers for it.

[brettp]

Please pull it in. The embed code was so rough I just aimed to get it working for 1.8.0 so any improvements would be great.

[cash]

Here is the branch I'm working in: ​https://github.com/cash/Elgg/tree/embed-menu

1. All the registration happens through the menu system
2. No more grabbing the raw menu items to create the tabs
3. I added a default list view so that a plugin can register a tab and tell the embed plugin to handle the display

I'm thinking of getting rid of the select and upload sections to the tabs. Using the menu system, plugins can specify the ordering. Seems more flexible.

I've got a little more work on this. It felt a little weird at first to register information on rendering the tab content with the menu item, but I guess the tab content goes with the actual tab of the menu...

I definitely want someone else to review before I pull it in.

[brettp]

Agreed to remove select vs upload sections.

Also agreed that it feels weird to register getter options through the menu system. I don't have a problem with it, I think it's bordering on what we can reasonably do with a "menu" system. Maybe in a future version we can abstract the menu system out to a generic registration system. There were early attempts to do something like that in the 1.0 days.

I think I like the use of elgg_set_config() for the current tab information, though we don't really do anything else like that in core / bundled plugins IIRC. Cleaner than stuffing into global $CONFIG.

Bonus points for updated README.

I think this is a good direction to go. It's hundreds of times better than what was there.

[tomv]

Tried to test it this evening. We are interested as it seems it would be easier to add tabs like 'Embed from Flickr' and to replace the embed image from URL, which is now part of Tinymce... ( The embed url menu icon in Tinymce should be removed there in our opinion. ... well.. think of it: that icon should open the embed dialog... instead of the somewhat out of place 'embed content' link above the editor. )

Attempt 1:

Renamed older mod/embed folder to mod/old_embed and uploaded the branch ... wow... WSOD, even admin section is broken. Quickly remove that folder again!

Attempt 2:

Remove mod/old_embed and create new mod/embed by uploading branch. Now we get modal dialog with : "The requested content cannot be loaded. Please try again later."

Attempt 3:

Leave old mod/embed and upload your branch over it... Now I'm getting

Embed content

Files
Upload a file

No items found.

Even though there are enough items to embed in the first place. I must admit, we are getting same message with current embed plugin, so the problem might be something else also...but no idea where...

Sorry... no help for you so far ))

[cash]

@Tom, you cannot grab just the embed plugin. You also have to grab the file plugin from my branch.

[tomv]

Ok, got it working by uploading both file and embed . Tried a few things and it works great - solid and simple.

In IE7, it displays tabs on different lines and after upload, window does not close, but still usable, so no reason to wait I think.

All in all: to me it seems a go :)

@Cash: Would you like us to put a little work in TinyMCE to remove the image insert button and as per ​http://www.tinymce.com/tryit/custom_toolbar_button.php add button that would replace the 'embed content' link?

[cash]

I have not tested it on IE7. We have decided to lower our support for IE7 to C status (​http://yuilibrary.com/yui/docs/tutorials/gbs/#c-grade). We'll fix enough issues so that it looks decent and works, but it won't have the exact look and feel for modern browsers.

Brett, Evan - thoughts on replacing the TinyMCE image insert button with the embed content link? I guess TinyMCE would unregister the menu item. It would also detect whether the embed plugin is on before turning off the image insert button.

[brettp]

Replying to cash:

Brett, Evan - thoughts on replacing the TinyMCE image insert button with the embed content link? I guess TinyMCE would unregister the menu item. It would also detect whether the embed plugin is on before turning off the image insert button.

Not a huge fan of it, especially since TinyMCE's embed image functionality is different (allows URLs, control class and style, etc). If we're going to integrate with TinyMCE's toolbar, I'd rather add a new icon than change the functionality of an existing one.

[tomv]

I agree we should add a new icon if the embed plugin is enabled.

Maybe in future we should make the list of icons a plugin setting/text field. Something like

"cut,copy,paste,|,link,unlink,anchor,image,elgg_embed"

, - just all standard TinyMCE stuff, except for the elgg_embed. Admins could decide to either remove or leave the 'image' icon. Very simple to implement and quite powerfull for specials...

[cash]

You can override the view js/tinymce to change the configuration of TinyMCE. I could see making certain features of TinyMCE configurable through plugin settings, but it would be down the priority list. We still have over 50 tickets open for the 1.8.1 release.

[tomv]

Understood. The embed icon is something we offered to contribute; certainly no intention to distract you from more important things.

[Cash Costello]

Merge pull request #70 from cash/embed-menu

Fixes #3852, #3854, #3855 Refs #3567 Embed plugin rewrite

Changeset: c1ae08ad43ef1eafbd92fc9526c2fdecbe5c5977

[Cash Costello]

Merge pull request #70 from cash/embed-menu

Fixes #3852, #3854, #3855 Refs #3567 Embed plugin rewrite

Changeset: c1ae08ad43ef1eafbd92fc9526c2fdecbe5c5977