[redesign] File page #963

2018-01-26T02:29:34+01:00

neb-b commented

2018-01-26 02:29:34 +01:00

(Migrated from github.com)

What I did:
Basic styling/layout for file page components

What I need to come back to:
The video player doesn't scale properly. I spent some time on it and was having issues, just going to leave it for now and come back later
Send tip modal (style)
Download progress/open/remove/report buttons inside of the file viewer

For the buttons inside the file viewer I didn't spend much time on them. I think more discussion is needed on if we actually want them there, and if so what the exact behavior should be.

What I did: Basic styling/layout for file page components What I need to come back to: The video player doesn't scale properly. I spent some time on it and was having issues, just going to leave it for now and come back later Send tip modal (style) Download progress/open/remove/report buttons inside of the file viewer For the buttons inside the file viewer I didn't spend much time on them. I think more discussion is needed on if we actually want them there, and if so what the exact behavior should be.

neb-b (Migrated from github.com) reviewed 2018-01-31 22:30:30 +01:00

src/renderer/component/common/spinner.jsx

					
				@ -1,14 +1,14 @@

				// @flow

neb-b (Migrated from github.com) commented

2018-01-31 22:30:30 +01:00

Changed the spinner to not actually be a spinner, but might as well keep the name as is

neb-b (Migrated from github.com) reviewed 2018-01-31 22:33:31 +01:00

src/renderer/component/video/internal/play-button.jsx

					
				@ -19,1 +5,3 @@

				  }

				type Props = {

				  play: string => void,

				  isLoading: boolean,

neb-b (Migrated from github.com) commented

2018-01-31 22:33:31 +01:00

Play/pause from the spacebar seems to work fine without this. Maybe it's need for other OS's?

neb-b (Migrated from github.com) reviewed 2018-01-31 22:36:28 +01:00

src/renderer/page/file/view.jsx

neb-b (Migrated from github.com) commented

2018-01-31 22:36:28 +01:00

I'll come back to this later

neb-b (Migrated from github.com) reviewed 2018-01-31 22:37:52 +01:00

src/renderer/scss/_gui.scss

neb-b (Migrated from github.com) commented

2018-01-31 22:37:52 +01:00

Not sure if we need this, I'll come back to it

liamcardenas (Migrated from github.com) reviewed 2018-02-01 21:56:57 +01:00

src/renderer/component/video/internal/play-button.jsx

					
				@ -19,1 +5,3 @@

				  }

				type Props = {

				  play: string => void,

				  isLoading: boolean,

liamcardenas (Migrated from github.com) commented

2018-02-01 21:55:50 +01:00

yeah it should be fine without this... ill check linux

liamcardenas (Migrated from github.com) commented

2018-02-01 21:56:56 +01:00

yeah it should be fine without this, I will check linux

liamcardenas (Migrated from github.com) reviewed 2018-02-01 22:37:57 +01:00

liamcardenas (Migrated from github.com) left a comment

As usual, I'm very happy with this. I have two comments about the code. And then I will post a follow up comment talking about the aesthetics.

Fine to merge at your convenience

As usual, I'm very happy with this. I have two comments about the code. And then I will post a follow up comment talking about the aesthetics. Fine to merge at your convenience

src/renderer/component/video/internal/play-button.jsx

					
				@ -19,1 +5,3 @@

				  }

				type Props = {

				  play: string => void,

				  isLoading: boolean,

liamcardenas (Migrated from github.com) commented

2018-02-01 22:31:21 +01:00

ok i figured our why it was there.

When you load the page, don't click anything, and press the space bar, this starts the video / initiates the download. When you take this out, it no longer works -- at least on linux.

That said, I think its better to take out this functionality, since we are going to implement autoplay soon anyway.

ok i figured our why it was there. When you load the page, don't click anything, and press the space bar, this starts the video / initiates the download. When you take this out, it no longer works -- at least on linux. That said, I think its better to take out this functionality, since we are going to implement autoplay soon anyway.

src/renderer/component/walletSendTip/view.jsx

					
				@ -21,0 +34,4 @@

				    const { claim_id: claimId, uri, sendSupport, sendTipCallback } = this.props;

				    const { amount } = this.state;

				    sendSupport(amount, claimId, uri);

liamcardenas (Migrated from github.com) commented

2018-02-01 22:34:25 +01:00

I dont think it really matters, but isn't the better way to do this to make a type for WalletSendTip?

liamcardenas commented

2018-02-01 22:41:04 +01:00

(Migrated from github.com)

@seanyesmunt I think the video player is too small, and the font "Published on ..." is too big and far away from the video title if that makes sense. Also maybe the line spacing in the title is too large.

@seanyesmunt I think the video player is too small, and the font "Published on ..." is too big and far away from the video title if that makes sense. Also maybe the line spacing in the title is too large. ![filepageredesignscreenshot](https://user-images.githubusercontent.com/5027235/35704782-6a1be2a6-0755-11e8-8943-554b1515a59d.png)

neb-b commented

2018-02-01 23:08:23 +01:00

(Migrated from github.com)

@liamcardenas Agreed the video player is too small. That is pretty much the min height. When you shrink the window down all the way that is what it will be. I spent some time trying to get it to scale/grow nicely as the width increases, but got mad and gave up.

Valid concerns, that will be something we can discuss with nicholas on a group call when I come back around to this.

@liamcardenas Agreed the video player is too small. That is pretty much the min height. When you shrink the window down all the way that is what it will be. I spent some time trying to get it to scale/grow nicely as the width increases, but got mad and gave up. Valid concerns, that will be something we can discuss with nicholas on a group call when I come back around to this.

neb-b (Migrated from github.com) reviewed 2018-02-01 23:11:21 +01:00

src/renderer/component/walletSendTip/view.jsx

					
				@ -21,0 +34,4 @@

				    const { claim_id: claimId, uri, sendSupport, sendTipCallback } = this.props;

				    const { amount } = this.state;

				    sendSupport(amount, claimId, uri);