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

Adds the new replace flow to the video block #19162

Merged
merged 3 commits into from
Jan 7, 2020

Conversation

draganescu
Copy link
Contributor

Description

There is a new replace flow offered by a component which is currently implemented in the Image Block. This PR implements the new flow in the Audio Block.

How has this been tested?

Tested locally.

Screenshots

Screenshot 2019-12-16 at 13 27 34

@draganescu draganescu added the [Block] Video Affects the Video Block label Dec 16, 2019
@draganescu draganescu self-assigned this Dec 16, 2019
@draganescu draganescu force-pushed the add/media-flow-video-block branch from 152aa55 to f923d8f Compare December 16, 2019 15:11
@draganescu draganescu removed their assignment Dec 19, 2019
Copy link
Member

@Soean Soean left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The replacement flow works great 👍. If we don't use the state editing anymore, we can remove it from this component.

@draganescu draganescu force-pushed the add/media-flow-video-block branch from f923d8f to 960fdaa Compare January 6, 2020 09:29
@draganescu
Copy link
Contributor Author

Hi @Soean I removed the useless state in 960fdaa. Thank you for reviewing all these!

@draganescu draganescu requested a review from Soean January 6, 2020 09:31
Copy link
Member

@Soean Soean left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works great.
We can remove some old comments. Maybe we can do it in another PR.

@@ -47,10 +47,6 @@ class VideoEdit extends Component {
super( ...arguments );
// edit component has its own src in the state so it can be edited
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The src state was removed in #6820, so we can remove this comment.

@@ -95,8 +90,7 @@ class VideoEdit extends Component {
const { attributes, setAttributes } = this.props;
const { src } = attributes;

// Set the block's src from the edit component's state, and switch off
// the editing UI.
// Set the block's src from the edit component's state
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We also can remove this comment, it's not valid since #6820

Copy link
Member

@Soean Soean left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works great, so we can merge it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Video Affects the Video Block
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants