search

kaltura / all-in-one-video-pack.wordpress

A Wordpress Plugin to simplify adding Kaltura to your Blog
https://wordpress.org/plugins/all-in-one-video-pack/
19 stars 31 forks source link

Remove unnecessary styling for html and body #83

Closed carlosesilva closed 6 years ago

carlosesilva commented 6 years ago

These styles cause the editor's header (and sometimes footer) to get absolute positioned in a weird spot when the post has enough content to trigger the sticky mode.

More information about this issue can be found here: #82

kaltura-hooks commented 6 years ago

Hi @carlosesilva, Thank you for contributing this pull request! Please sign the Kaltura CLA so we can review and merge your contribution. Learn more at http://bit.ly/KalturaContrib

carlosesilva commented 6 years ago

Hello there,

CLA signed =]

rkreich commented 6 years ago

Thanks for this fix @carlosesilva. I think admin.css was originally included in admin modal boxes (used standalone iframes) and somehow the file found itself being added on all admin pages. height: 100% is a legacy rule but overflow: auto was added to fix mobile scrolling. Can you verify that scrolling is not affect on upload and "insert into post" screens (on mobile device)?

carlosesilva commented 6 years ago

Now that you mention it, I did notice that yes admin.css is affecting all of wp-admin and I just found an issue in mobile (see my iphone screenshot below). It could also have other side effects that we may not have noticed yet.

image Notice how the side margins are messed up

carlosesilva commented 6 years ago

I am ok with keeping the stylesheet as is, as long as it is only being applied inside an iframe.

I think the solution here would be to only register the kaltura-admin css and js files in the adminEnqueueScripts method but only enqueue them inside your controllers that need them. Like you already do here:

https://github.com/kaltura/all-in-one-video-pack.wordpress/blob/master/lib/Kaltura/LibraryController.php#L28

rkreich commented 6 years ago

@carlosesilva your solution sounds right. Will you be able to update this PR? Thanks

carlosesilva commented 6 years ago

Hi @rkreich,

I have pushed some new commits that prevent the kaltura js/css admin files from being loaded on every admin page, only loading it when necessary.

Let me know what you think =]

Thanks, Carlos

carlosesilva commented 6 years ago

Hi @rkreich,

I don't think there is any controller running on the edit post screens automatically so I moved the enqueues to the editor_form_top action hook. Let me know if you think there is a better hook to be used here.

rkreich commented 6 years ago

Thanks @carlosesilva, I've merged your PR