Video overlay #825

Closed
hackrush01 wants to merge 9 commits from video-overlay into master
hackrush01 commented 2017-12-07 19:10:12 +01:00 (Migrated from github.com)
No description provided.
neb-b (Migrated from github.com) reviewed 2017-12-08 06:17:24 +01:00
@ -0,0 +205,4 @@
return (
<div className="video__loading-screen">
<div>
{spinner && <div className="video__loading-spinner" />}
neb-b (Migrated from github.com) commented 2017-12-08 06:17:24 +01:00

This should use the <Spinner /> component

This should use the `<Spinner />` component
hackrush01 (Migrated from github.com) reviewed 2017-12-08 11:24:38 +01:00
@ -0,0 +205,4 @@
return (
<div className="video__loading-screen">
<div>
{spinner && <div className="video__loading-spinner" />}
hackrush01 (Migrated from github.com) commented 2017-12-08 11:24:38 +01:00

I noticed one thing. We already have a common.js file here(file 1), but the <Spinner /> component comes from here(folder 1), a new common folder with just that component.
Are we breaking down file 1 to individual components and putting them in folder 1? Or should I include <Spinner /> component in file 1. The latter option is better IMO as then we don't have to include different files for each "common" component, unless former is better for some use cases.

I noticed one thing. We already have a `common.js` file [here(file 1)](https://github.com/lbryio/lbry-app/blob/master/src/renderer/js/component/common.js), but the `<Spinner />` component comes from [here(folder 1)](https://github.com/lbryio/lbry-app/blob/master/src/renderer/js/component/common), a new common folder with just that component. Are we breaking down file 1 to individual components and putting them in folder 1? Or should I include `<Spinner />` component in file 1. The latter option is better IMO as then we don't have to include different files for each "common" component, unless former is better for some use cases.
kauffj (Migrated from github.com) reviewed 2017-12-08 17:03:16 +01:00
@ -0,0 +205,4 @@
return (
<div className="video__loading-screen">
<div>
{spinner && <div className="video__loading-spinner" />}
kauffj (Migrated from github.com) commented 2017-12-08 17:03:16 +01:00

@hackrush01 @seanyesmunt @liamcardenas I think common.js should die and these should all just be individual components.

@hackrush01 @seanyesmunt @liamcardenas I think `common.js` should die and these should all just be individual components.
liamcardenas (Migrated from github.com) reviewed 2017-12-08 18:33:06 +01:00
@ -0,0 +205,4 @@
return (
<div className="video__loading-screen">
<div>
{spinner && <div className="video__loading-spinner" />}
liamcardenas (Migrated from github.com) commented 2017-12-08 18:33:06 +01:00

I agree

I agree
lyoshenka commented 2018-02-05 21:48:55 +01:00 (Migrated from github.com)

This is on hold. When we're ready to work on this, we should re-evaluate and figure out the right approach.

This is on hold. When we're ready to work on this, we should re-evaluate and figure out the right approach.

Pull request closed

Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
LBRYCommunity/lbry-desktop!825
No description provided.