Page MenuHomePhabricator

Improve trimming UI
Closed, ResolvedPublic

Assigned To
Authored By
Khr2003
Mar 29 2021, 7:07 AM
Referenced Files
F34296167: Screenshot from 2021-04-08 23-20-16.png
Apr 8 2021, 5:53 PM
F34196104: image.png
Mar 29 2021, 12:03 PM
F34195679: 1617010574311.png
Mar 29 2021, 9:47 AM
F34195549: image.png
Mar 29 2021, 9:18 AM

Description

Improve UI for trimming functionality to make it easier for users to trim. It will show the total duration and thumbnails of the video in a timeline.

Event Timeline

Khr2003 renamed this task from Improve trim UI to Improve trimming UI.Mar 29 2021, 7:38 AM
Khr2003 updated the task description. (Show Details)

Change 675453 had a related patch set uploaded (by Khr2003; author: Khr2003):
[labs/tools/VideoCutTool@master] Improve trimming function UI

https://gerrit.wikimedia.org/r/675453

Extremely Amazing work! @Khr2003, Yes this would definitely help users trim smoothly :)) Couple of pointers from my side.

  • As the trim layer overlapped with the video player, I'm unable to watch the video and select the trim range at the same time. So I need to minimize the trim layer, watch the video and then re-open the trim layer again.
  • How about we also enable the input buttons for the trim range selection? Because some users might not drag the trim layer precisely so they can change little values if the input boxes are also available?
  • Just a thought, what do think about having the trim section and player horizontal width the same?
  • Try to remove the warnings in the console.

image.png (921×1 px, 586 KB)

Your great work has resulted in tangible, beneficial results of the tool. Good work, as always!

Thanks for the comment. I am glad it was useful :)

  • As the trim layer overlapped with the video player, I'm unable to watch the video and select the trim range at the same time. So I need to minimize the trim layer, watch the video and then re-open the trim layer again.

I was not aware of that. On my screen it was a bit different. They overlapped but not this much.

  • How about we also enable the input buttons for the trim range selection? Because some users might not drag the trim layer precisely so they can change little values if the input boxes are also available?

I thought of that at first. But I thought the method I added was little better. If you resize the range (either left or right) for 700ms two arrows will appear for precise selection.

1617010574311.png (977×1 px, 409 KB)

  • Just a thought, what do think about having the trim section and player horizontal width the same?

I thought of adding it inside the settings sidebar but then it will not be a great user experience. Thumbnails will not be very visible and users will have limited space to adjust the handles. I think the whole UI needs changing so it feels more like a video editor not a website. I can work on that and show you some suggestions but it might take some time.

  • Try to remove the warnings in the console.

The new changes cause one error only Warning: Cannot update during an existing state transition (such as within render). Render methods should be a pure function of props and state.. I tried to resolve it but the state stopped updating. I will try to figure out a way to resolve while state is being updated.

I thought of adding it inside the settings sidebar but then it will not be a great user experience. Thumbnails will not be very visible and users will have limited space to adjust the handles. I think the whole UI needs changing so it feels more like a video editor not a website. I can work on that and show you some suggestions but it might take some time.

Nah nah what I what was meaning is, Adding the trim section just below the video player such that trim section width and player width will be equal. But yeah I don't think doing this way will be very visible to users as per your comments above, yeah.

Nah nah what I what was meaning is, Adding the trim section just below the video player such that trim section width and player width will be equal. But yeah I don't think doing this way will be very visible to users as per your comments above, yeah.

I understand now. The user will need to scroll up and down between the range and the video player.

So what changes do I have to do now. If the current changes are ok we can push it and then I can work on modifying the whole UI so it is a single page app. Scrolling will be only for parts of the page.

Before merging the code, I have couple of points as well.

  • Fixing the overlap issue with the player.
  • How about the functionality to change the trim slider pointers with keyboard left and right arrows?
  • I tested this patch in windows OS with Node 12^ and linux OS with Node (10.19.0). Tool is superb cool in the windows but in Linux system the behavior is not working as expected, if I click on any buttons in the trim layer it's returning some errors in the browser. These errors are same in both chrome and firefox.
    image.png (447×1 px, 142 KB)

PS: The instance which we are deploying in server is also a Linux system of Node version 10.23.1.

  • Fixing the overlap issue with the player.

I will work on that.

  • How about the functionality to change the trim slider pointers with keyboard left and right arrows?

I thought of this but decided not to do so because the new code became large (more than 700 lines) and also mobile users will not benefit from that. However, I can implement as it will not take much time to develop.

  • I tested this patch in windows OS with Node 12^ and linux OS with Node (10.19.0). Tool is superb cool in the windows but in Linux system the behavior is not working as expected, if I click on any buttons in the trim layer it's returning some errors in the browser. These errors are same in both chrome and firefox.
    image.png (447×1 px, 142 KB)

I don't have a linux system but I think it is time for me to get one. I will test it on a linux system and see what the issue is.

I don't have a linux system but I think it is time for me to get one. I will test it on a Linux system and see what the issue is.

I doubt if it's just the problem with the Node version irrespective of the OS? Not sure though.

@Gopavasanth
I updated the code. I tested the code on Node 10.19.0 and it worked fine with no issues. (I tested it on windows because I could not install version 10.19 on linux).

I made the trim box smaller so it overlaps with the video player as less as possible.

I did not apply the keyboard shortcuts as it turned out to be a bigger job that I initially thought. I will try to add it in a separate task.

Hey @Khr2003, In my local system, I'm still facing some errors :/
Browser: Firefox

Screenshot from 2021-04-08 23-20-16.png (708×1 px, 106 KB)

@Khr2003, Can you try to deploy in Heroku or in any free hosting service and give the link so that we can test in multiple devices if that works in production?

@Gopavasanth I tried to deploy it but received this error:

import React from 'react';
       ^^^^

SyntaxError: Unexpected identifier

@Khr2003, How are you deploying the tool? Did you try deploying the static pages (build directory) after running the npm run build in the repo directory?

Hi @Khr2003,

Please avoid the trim section overlap with the player so that we should be ready to merge the patch.

@Khr2003, Do you have any updates on your patch?

Change 675453 abandoned by Gopavasanth:

[labs/tools/VideoCutTool@master] Improve trimming function UI

Reason:

As source code is updated.

https://gerrit.wikimedia.org/r/675453