Page MenuHomePhabricator

Split home.js into different components
Open, Needs TriagePublic

Description

Currently, home.js looks too large and became difficult to debug and solve issues so split home.js into different components.

Event Timeline

@Gopavasanth Hi! As this task is tagged with good first task, it might be ideal if you can add to the task description, what home.js component does, and how to go about splitting it. For the scope of this task, contributing to one new component might be reasonable that you can advise what that should be. Please update the task description accordingly. Thanks!

Can I do this task, in the direction of contributing for the project idea https://phabricator.wikimedia.org/T164307 ?

We can split the code into many components for instance Header, Footer, Player, VideoSettings (menu) and many more, For the acceptance of the code, at least one component should be made without any code duplication.

Hello @Hsync7 If you are working on this task, Please claim the task and feel free to ask any questions if you have !

Soda added a subscriber: Soda.

I'll try to get some part of this done.

Change 630981 had a related patch set uploaded (by Sohom Datta; owner: Sohom Datta):
[labs/tools/VideoCutTool@master] Seperated header, footer, notificationsUtils and VideoSettings

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

Change 630981 merged by jenkins-bot:
[labs/tools/VideoCutTool@master] Seperated header, footer, notificationsUtils and VideoSettings

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

Gopavasanth removed Soda as the assignee of this task.EditedJan 19 2021, 5:50 PM

Thanks, @Soda for your initial commit, As this ticket, can have many instances removing the assignee so anyone else can also work on this, please feel free to create another patch if you are interested to work on this ticket again :)

Hi @Gopavasanth. I'm a first-time contributor at WikiMedia. The file seems humongous. Can you help me out by suggesting some lines that can be moved and I'll move them initially? My initial thought is to move some functions into another directory called "utils" and then import them. Does that sound like something that'll help? Thanks!

@SarthakKundra Welcome! Yes, that sounds like a great plan and you can go ahead with that :)

Hi @Gopavasanth. Getting the following error on running npm run start. Can you help me with this? I did run npm audit fix to fix the vulnerabilities caused by old packages being used.

Hi @SarthakKundra, Can you try removing the node_modules directory and re-install them again with npm install?

Hi @SarthakKundra, Can you try removing the node_modules directory and re-install them again with npm install?

Already tried that twice. First i thought it was because I did it on a different node version. Then did it after switching to the correct node version using nvm

@Gopavasanth For some reason a fresh clone seems to work fine. Can you please guide me about the process of creating a PR? Should I create it on the Github mirror only? If not, it'll be great if you can guide me a bit. Thanks!

@Gopavasanth I submitted a patch on Gerrit. How do I link it here?
CC :- @Aklapper

@Aklapper I've already submitted the patch. Should I change it's commit message?

Done @Aklapper although I can't see it being commented here by a bot. Maybe it'll take some time.

Change 660995 had a related patch set uploaded (by SarthakKundra; owner: SarthakKundra):
[labs/tools/VideoCutTool@master] Refactor:: Move formatTime and decodeTime functions from home.js to a different file

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

Hi @SarthakKundra are you sure you tested your patch locally and it works? My console gives

Failed to compile.

./src/components/home.js
  Line 1166:59:  'decodeTime' is not defined  no-undef
  Line 1211:59:  'decodeTime' is not defined  no-undef

Search for the keywords to learn more about each error.

Change 662079 had a related patch set uploaded (by SarthakKundra; owner: SarthakKundra):
[labs/tools/VideoCutTool@master] Refactor:: Moved formatTime and decodeTime functions to utils/time.js

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

Change 662080 had a related patch set uploaded (by SarthakKundra; owner: SarthakKundra):
[labs/tools/VideoCutTool@master] Refactor:: Moved formatTime and decodeTime functions to utils/time.js

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

Change 662112 had a related patch set uploaded (by SarthakKundra; owner: SarthakKundra):
[labs/tools/VideoCutTool@master] Refactor:: Move formatTime and decodeTime functions to utils/time.js

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

Change 662660 had a related patch set uploaded (by SarthakKundra; owner: SarthakKundra):
[labs/tools/VideoCutTool@master] Refactor:: Move formatTime and decodeTime functions to utils/time.js

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

Change 660995 abandoned by SarthakKundra:
[labs/tools/VideoCutTool@master] Refactor:: Moved formatTime and decodeTime functions to utils/time.js

Reason:
Fixed flaws in a new PR

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

Hi @Gopavasanth . I would like to work on this task . Is this task still open ? Thanks !

Hey @Uditdesai2206 I'm actively working on this. It'll be great to take up another task to avoid conflicting patches. Thanks :)

Change 662660 merged by jenkins-bot:
[labs/tools/VideoCutTool@master] Refactor:: Move formatTime and decodeTime functions to utils/time.js

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

Change 662112 abandoned by Gopavasanth:
[labs/tools/VideoCutTool@master] Refactor:: Move formatTime and decodeTime functions to utils/time.js

Reason:
Suppressed by https://gerrit.wikimedia.org/r/c/labs/tools/VideoCutTool/ /662660/

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

Change 662080 abandoned by Gopavasanth:
[labs/tools/VideoCutTool@master] Refactor:: Moved formatTime and decodeTime functions to utils/time.js

Reason:
Suppressed by https://gerrit.wikimedia.org/r/c/labs/tools/VideoCutTool/ /662660/

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

Change 662079 abandoned by Gopavasanth:
[labs/tools/VideoCutTool@master] Refactor:: Moved formatTime and decodeTime functions to utils/time.js

Reason:
Suppressed by https://gerrit.wikimedia.org/r/c/labs/tools/VideoCutTool/ /662660/

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