Improve subscriptions page #2031

Merged

neb-b merged 2 commits from subscription-new into master

2018-10-22 22:31:55 +02:00

neb-b commented

2018-10-12 21:03:39 +02:00

(Migrated from github.com)

Notes

It's not as bad as the diff numbers suggest. I added a lot of flow type checks
The entire subscriptions code is now covered by flow (and all of it was moved to it's own file instead of living in the reducer, which I like a lot more)

Changes

Changed wording around subscriptions
- Notifications => Unread
- This was mostly to avoid confusion in the future with actual notifications
Unread subscriptions aren't "marked as read" until you navigate to the File page
- Previously all files were "marked as read" when you navigated to the subscriptions page
- It is possible to mark an entire channel as read with this code, but that wasn't add in this PR
New content on the subscriptions page will have a NEW badge
There are now two views
- View all
  - Lists the first page of all of your subscriptions
- View latest first
  - Lists only unread subscriptions, seperated by channel, and where the channel with the newest content first is at the top
  - In a future PR, we ill show recommended subscriptions when there are no new subscriptions on this view

Pictures

### Notes It's not as bad as the diff numbers suggest. I added a lot of flow type checks The entire subscriptions code is now covered by flow (and all of it was moved to it's own file instead of living in the reducer, which I like a lot more) ## Changes - Changed wording around subscriptions - Notifications => Unread - This was mostly to avoid confusion in the future with actual notifications - Unread subscriptions aren't "marked as read" until you navigate to the File page - Previously all files were "marked as read" when you navigated to the subscriptions page - It is possible to mark an entire channel as read with this code, but that wasn't add in this PR - New content on the subscriptions page will have a `NEW` badge - There are now two views - View all - Lists the first page of all of your subscriptions - View latest first - Lists only unread subscriptions, seperated by channel, and where the channel with the newest content first is at the top - In a future PR, we ill show recommended subscriptions when there are no new subscriptions on this view ## Pictures <img width="500" alt="screen shot 2018-10-17 at 5 46 17 pm" src="https://user-images.githubusercontent.com/16882830/47132169-9b641e00-d270-11e8-9397-1a43c78018bf.png"> <img width="500" alt="screen shot 2018-10-17 at 5 46 07 pm" src="https://user-images.githubusercontent.com/16882830/47132173-a1f29580-d270-11e8-9631-42e6c0c4c227.png"> <img width="500" alt="screen shot 2018-10-17 at 5 52 40 pm" src="https://user-images.githubusercontent.com/16882830/47132179-a6b74980-d270-11e8-9a8c-2f4f063d595d.png">

neb-b commented

2018-10-18 06:55:18 +02:00

(Migrated from github.com)

@skhameneh This is ready for review. I still have to make some very minor css changes for colors/the new badge styling but all of the JS is ready to be looked over.

skhameneh commented

2018-10-18 08:43:01 +02:00

(Migrated from github.com)

Looks good, I don't see anything that sticks out; I'll go over it better tomorrow.

skhameneh (Migrated from github.com) reviewed 2018-10-19 07:58:48 +02:00

src/renderer/index.js

skhameneh (Migrated from github.com) commented

2018-10-19 07:17:01 +02:00

You can place this in your entrypoint array

src/renderer/page/subscriptions/view.jsx

skhameneh (Migrated from github.com) commented

2018-10-19 07:18:52 +02:00

Seems like this logic should be in a utility

skhameneh (Migrated from github.com) commented

2018-10-19 07:45:43 +02:00

This is not very intuitive, I would suggest using a reduce; the logic reads very odd since it looks to be a boolean operation but returns a value.

skhameneh (Migrated from github.com) commented

2018-10-19 07:48:53 +02:00

Maybe

allSubscriptions.reduce(({ name, claim_id: claimId }, index, arr) => {
  name && claimId && arr.push(`lbry://${name}#${claimId}`)
}, []);

Up to you

Maybe ``` allSubscriptions.reduce(({ name, claim_id: claimId }, index, arr) => { name && claimId && arr.push(`lbry://${name}#${claimId}`) }, []); ``` Up to you

src/renderer/redux/actions/content.js

skhameneh (Migrated from github.com) commented

2018-10-19 07:50:17 +02:00

I see this duplicated a number of times

src/renderer/redux/selectors/subscriptions.js

skhameneh (Migrated from github.com) commented

2018-10-19 07:54:49 +02:00

Should this be a boolean?

skhameneh (Migrated from github.com) commented

2018-10-19 07:55:08 +02:00

indent

skhameneh (Migrated from github.com) commented