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

Add video support on the editor #3503

Merged
merged 29 commits into from Apr 14, 2015

Conversation

Projects
None yet
2 participants
@SergioEstevao
Contributor

SergioEstevao commented Apr 6, 2015

Add support for video insertion on the visual editor.

cc @bummytime

@bummytime

This comment has been minimized.

Show comment
Hide comment
@bummytime

bummytime Apr 8, 2015

Member

@SergioEstevao a couple of general things:

  1. We should probably update dialogs so it says "videos" or "media" (cc @meremagee):
    napkin 04-08-15 1 31 26 pm

  2. Videos are not being displayed when creating new posts and editing existing posts. Looks like they are being saved to the server as a span:

<span id="video_container_1A9706BF-7924-400A-8F7F-E45F8ACD0BAC" class="video_container"></span>
  1. At times, noticed some wacky behavior around images when inserting newlines and/or deleting text/videos with backspace. For example the text would flow under the placeholder/poster image. I think this is related to the 2nd point above.

  2. When deleting a video by backspacing over it, the nav bar "uploading" progress bar still displays: https://cloudup.com/cBmDmgY8RAD

Member

bummytime commented Apr 8, 2015

@SergioEstevao a couple of general things:

  1. We should probably update dialogs so it says "videos" or "media" (cc @meremagee):
    napkin 04-08-15 1 31 26 pm

  2. Videos are not being displayed when creating new posts and editing existing posts. Looks like they are being saved to the server as a span:

<span id="video_container_1A9706BF-7924-400A-8F7F-E45F8ACD0BAC" class="video_container"></span>
  1. At times, noticed some wacky behavior around images when inserting newlines and/or deleting text/videos with backspace. For example the text would flow under the placeholder/poster image. I think this is related to the 2nd point above.

  2. When deleting a video by backspacing over it, the nav bar "uploading" progress bar still displays: https://cloudup.com/cBmDmgY8RAD

@SergioEstevao

This comment has been minimized.

Show comment
Hide comment
@SergioEstevao

SergioEstevao Apr 11, 2015

Contributor

@bummytime

  1. Is fixed by this change:b248955
  2. Is fixed by this change:wordpress-mobile/WordPress-Editor-iOS#626
  3. I think it's fixed by the fix in 2 but you may want to test it again
  4. This is something that also happens with images they are just quicker to finish the upload. I had a look but there is no easy way to find out that the video was deleted from the post. The only I see it happen is by constantly checking the post content to see if the video tag was deleted and stop the upload for that image. Maybe this should be a new improvement issue?
Contributor

SergioEstevao commented Apr 11, 2015

@bummytime

  1. Is fixed by this change:b248955
  2. Is fixed by this change:wordpress-mobile/WordPress-Editor-iOS#626
  3. I think it's fixed by the fix in 2 but you may want to test it again
  4. This is something that also happens with images they are just quicker to finish the upload. I had a look but there is no easy way to find out that the video was deleted from the post. The only I see it happen is by constantly checking the post content to see if the video tag was deleted and stop the upload for that image. Maybe this should be a new improvement issue?
@bummytime

This comment has been minimized.

Show comment
Hide comment
@bummytime

bummytime Apr 11, 2015

Member

Looking better @SergioEstevao!

This is something that also happens with images they are just quicker to finish the upload. I had a look but there is no easy way to find out that the video was deleted from the post. The only I see it happen is by constantly checking the post content to see if the video tag was deleted and stop the upload for that image. Maybe this should be a new improvement issue?

I can open a new issue for this.

Also, regarding deleting videos. I have another vid for you: https://cloudup.com/cjyvInV7SZi Seems like the video will not be removed until the surrounding &nbsp;'s are deleted. Also, if the &nbsp; in front of a video is removed, the video cannot be deleted (unless the user is in HTML mode).

Member

bummytime commented Apr 11, 2015

Looking better @SergioEstevao!

This is something that also happens with images they are just quicker to finish the upload. I had a look but there is no easy way to find out that the video was deleted from the post. The only I see it happen is by constantly checking the post content to see if the video tag was deleted and stop the upload for that image. Maybe this should be a new improvement issue?

I can open a new issue for this.

Also, regarding deleting videos. I have another vid for you: https://cloudup.com/cjyvInV7SZi Seems like the video will not be removed until the surrounding &nbsp;'s are deleted. Also, if the &nbsp; in front of a video is removed, the video cannot be deleted (unless the user is in HTML mode).

@SergioEstevao

This comment has been minimized.

Show comment
Hide comment
@SergioEstevao

SergioEstevao Apr 12, 2015

Contributor

Also, regarding deleting videos. I have another vid for you: https://cloudup.com/cjyvInV7SZi Seems like the video will not be removed until the surrounding  's are deleted. Also, if the   in front of a video is removed, the video cannot be deleted (unless the user is in HTML mode).

I've being trying to sort this out but it looks something that is part of the edit mode of the webview. If you add the video between text it works ok. The issues only show up if you have a empty post or add a video to the end.
I've been trying different characters to wrap around the video but they all end up with the same result.

Contributor

SergioEstevao commented Apr 12, 2015

Also, regarding deleting videos. I have another vid for you: https://cloudup.com/cjyvInV7SZi Seems like the video will not be removed until the surrounding  's are deleted. Also, if the   in front of a video is removed, the video cannot be deleted (unless the user is in HTML mode).

I've being trying to sort this out but it looks something that is part of the edit mode of the webview. If you add the video between text it works ok. The issues only show up if you have a empty post or add a video to the end.
I've been trying different characters to wrap around the video but they all end up with the same result.

@bummytime

This comment has been minimized.

Show comment
Hide comment
@bummytime

bummytime Apr 13, 2015

Member

@meremagee @thianhlu Take a look at @SergioEstevao's comment above and watch the video. Is this a showstopper?

Member

bummytime commented Apr 13, 2015

@meremagee @thianhlu Take a look at @SergioEstevao's comment above and watch the video. Is this a showstopper?

@bummytime

This comment has been minimized.

Show comment
Hide comment
@bummytime

bummytime Apr 13, 2015

Member

@SergioEstevao Just an update....I spoke with @meremagee offline and we both agreed that this delete issue can be ignored for this PR. When we finally merge, lets create another issue for it.

Member

bummytime commented Apr 13, 2015

@SergioEstevao Just an update....I spoke with @meremagee offline and we both agreed that this delete issue can be ignored for this PR. When we finally merge, lets create another issue for it.

@bummytime

This comment has been minimized.

Show comment
Hide comment
@bummytime

bummytime Apr 13, 2015

Member

@SergioEstevao after wordpress-mobile/WordPress-Editor-iOS#626 is merged and the editor hash is updated here, I say :shipit: ...let's get this thing into stabilization!

Nice job on this!

Member

bummytime commented Apr 13, 2015

@SergioEstevao after wordpress-mobile/WordPress-Editor-iOS#626 is merged and the editor hash is updated here, I say :shipit: ...let's get this thing into stabilization!

Nice job on this!

SergioEstevao pushed a commit that referenced this pull request Apr 14, 2015

@SergioEstevao SergioEstevao merged commit 99ddacf into develop Apr 14, 2015

1 check was pending

continuous-integration/travis-ci/push The Travis CI build is in progress
Details

@SergioEstevao SergioEstevao deleted the issue/Editor-14-MediaAddVideo branch Apr 14, 2015

@bummytime bummytime referenced this pull request Apr 23, 2015

Closed

Re-enable support for uploading videos #14

3 of 3 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment