-
Notifications
You must be signed in to change notification settings - Fork 8
Conversation
Should I create a trac issue for #102 as it still is broken in core media flows? |
@@ -82,11 +113,12 @@ | |||
metadata = control.mapModelToMediaFrameProps( control.model.toJSON() ); | |||
|
|||
// Set up the media frame. | |||
mediaFrame = wp.media({ | |||
mediaFrame = new AudioDetailsMediaFrame({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Having to extend the entire class to remove the replace-audio
seems like it should be unnecessary. It seems like all we really should have to do is:
mediaFrame.states.remove( 'replace-audio' );
But it's not working. What also doesn't work is:
mediaFrame.toolbar.view.states.remove( 'replace-audio' )
Nor does this work:
mediaFrame.states.get( 'replace-audio' ).deactivate()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ugh that is too bad the other approaches don't work. It would be nice if we could also pass an option
for states
to the wp.media
constructor too.
@@ -47,7 +47,8 @@ | |||
.media-frame.media-widget .image-details .embed-media-settings .setting.align, | |||
.media-frame.media-widget .attachment-display-settings .setting.align, | |||
.media-frame.media-widget .embed-media-settings .setting.align, | |||
.media-frame.media-widget .embed-link-settings .setting.link-text { | |||
.media-frame.media-widget .embed-link-settings .setting.link-text, | |||
.media-frame.media-widget .replace-attachment { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no underlying state that controls whether or not the Replace button is shown. So we have to rely on CSS instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree it isn't optimal to have to extend
to replace the states, but at least it is quite explicit reading the code what all is going on.
This is testing out well for me.
@@ -82,11 +113,12 @@ | |||
metadata = control.mapModelToMediaFrameProps( control.model.toJSON() ); | |||
|
|||
// Set up the media frame. | |||
mediaFrame = wp.media({ | |||
mediaFrame = new AudioDetailsMediaFrame({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ugh that is too bad the other approaches don't work. It would be nice if we could also pass an option
for states
to the wp.media
constructor too.
Fixes #102
Fixes #100