WordPress.org

Make WordPress Core

Opened 5 years ago

Last modified 2 years ago

#31139 reopened enhancement

Allow editing of video embed parameters in the media modal

Reported by: melchoyce Owned by: wonderboymusic
Milestone: Future Release Priority: normal
Severity: normal Version:
Component: Media Keywords: needs-patch
Focuses: ui Cc:
PR Number:

Description

Unsure if this is possible, but it would be pretty sweet if you could adjust available video embed parameters, such as height/width, straight from the media modal. It could fit pretty well on this screen: https://cloudup.com/caoYOVuWM41

Attachments (5)

31139.diff (6.6 KB) - added by wonderboymusic 5 years ago.
31139.2.diff (9.6 KB) - added by wonderboymusic 5 years ago.
31139.3.diff (9.6 KB) - added by wonderboymusic 5 years ago.
31139.4.diff (507 bytes) - added by wonderboymusic 5 years ago.
31139.patch (9.3 KB) - added by iseulde 5 years ago.

Download all attachments as: .zip

Change History (37)

#1 @SergeyBiryukov
5 years ago

  • Component changed from Embeds to Media

#2 @wonderboymusic
5 years ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to Future Release

@wonderboymusic
5 years ago

#3 @wonderboymusic
5 years ago

  • Keywords has-patch ui-feedback added; needs-patch removed
  • Milestone changed from Future Release to 4.2

The attached code works for editing embeds, but needs a cleaner UI

#4 @wonderboymusic
5 years ago

31139.2.diff is a better UI and mostly works, only problem being: there are some cases were we need to transition from a URL to an [embed] when the width and/or height changes. This currently doesn't work because of how MCE views find themselves in the DOM. Will ping @iseulde

This ticket was mentioned in Slack in #core by wonderboymusic. View the logs.


5 years ago

#6 @iseulde
5 years ago

I'll try to fix this in the next patch. We should auto detect the new view text and update everything accordingly. Currently you can't change the view's type.

#7 @melchoyce
5 years ago

Can someone with the patch applied post a screenshot? Thanks!

#9 @melchoyce
5 years ago

Thanks!

#10 @iseulde
5 years ago

This currently doesn't work because of how MCE views find themselves in the DOM. Will ping @iseulde

@wonderboymusic Tweaked the API. :) Haven't looked at the encoding problem though, but will fix!

#11 @wonderboymusic
5 years ago

In 31620:

Allow inline editing of width and height parameters while previewing an embed in the media modal:

  • Use wp.shortcode() instead of manually constructing a shortcode in views/embed/link
  • Allow a URL to transition to a shortcode (and vice versa) when returning an embed to TinyMCE
  • In WP_Embed, store the last URL and last set of attributes requested in class properties
  • wp_ajax_parse_embed(), allow [embed]s to have attributes. Return attr in the response.

This is a first pass to allow broad testing with recent MCE view changes.

See #31139.

#12 @wonderboymusic
5 years ago

In 31626:

After [31620], when checking for HTTP URLs, make sure we are checking the shortcode body instead of an indexed attribute.

See #31139.

#13 @helen
5 years ago

In 31642:

Media: UI tweaks for Insert from URL.

"Title" field is now labeled as "Link Text", to reflect how it's actually used. It's also hidden whenever the embed is updating, to make it clearer that something is happening. Embed dimension fields are shown below the preview, much like image property fields are shown below.

fixes #29476, see #31139.

#14 @DrewAPicture
5 years ago

  • Keywords 4.2-beta added
  • Owner set to wonderboymusic
  • Status changed from new to assigned

This will ride into Beta 1, see [31620], [31626], and [31642].

#15 @helen
5 years ago

Do we have any problems here with recent view changes, and/or is the encoding issue fixed now? Not sure if anything else was left here.

This ticket was mentioned in Slack in #core by drew. View the logs.


5 years ago

#17 @wonderboymusic
5 years ago

[31819] and [31817] completely broke embeds. Reverting them makes everything work fine. #dark

This ticket was mentioned in Slack in #core by wonderboymusic. View the logs.


5 years ago

#19 @helen
5 years ago

@wonderboymusic Is that something that needs to be fixed in this ticket or is this one done?

#20 @wonderboymusic
5 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

This ticket was mentioned in Slack in #core by iseulde. View the logs.


5 years ago

#22 @iseulde
5 years ago

Remaining issues fixed in [31832].

#23 @helen
5 years ago

  • Keywords revert added; has-patch ui-feedback 4.2-beta removed
  • Resolution fixed deleted
  • Status changed from closed to reopened

Per #32006 and as confirmed in testing, this isn't working in 4.2-RC3. Let's revert and try again later, perhaps with a more unified-feeling look. Right now they just kind of hang out under the video and you don't even see the fields half the time.

@iseulde
5 years ago

#24 @azaozz
5 years ago

  • Keywords commit added

31139.patch works well here.

#25 @DrewAPicture
5 years ago

  • Keywords changed from revert commit to revert commit revert

31139.patch looks good.

#26 @azaozz
5 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 32258:

Revert editing of video embed parameters in the media modal, [31620] and [31626] for now. Plan on revisiting in 4.3.
Props iseulde. Fixes #31139, fixes #32006.

#27 @helen
5 years ago

  • Keywords revert commit revert removed
  • Milestone changed from 4.2 to Future Release
  • Resolution fixed deleted
  • Status changed from closed to reopened

#28 @iseulde
5 years ago

#32035:

Not sure the current usage of _.debounce is correct when embedding a link from "Insert from URL". To my understanding, the updateoEmbed function should not be debounced and fetch should be debounced instead.
See discussion about _.debounce on #26600

#29 @iseulde
5 years ago

  • Keywords needs-patch added

#30 @iseulde
5 years ago

Sorry, this was not reverted. Ignore the link.

#31 @wonderboymusic
5 years ago

In 32330:

After [32258], restore the parts of [31620] and [31626] that weren't changes to the UI, but were improvements to existing code.

  • Use wp.shortcode() instead of manually constructing a shortcode in views/embed/link
  • In WP_Embed, store the last URL and last set of attributes requested in class properties
  • wp_ajax_parse_embed(), allow [embed]s to have attributes. Return attr in the response.

See #31139.

#32 @melchoyce
2 years ago

@matias @joen Do you think we can close this now that Gutenberg has the Inspector for media embeds?

Note: See TracTickets for help on using tickets.