Reader Lists: list followed tags #777

bluefuton · 2015-11-25T15:55:22Z

This PR adds a scrollable list of a Reader list's existing followed tags.

Fixes #637.

To test

Choose an existing Reader list that you own, and click 'Manage' in the sidebar.

Navigate to the 'Tags' tab, and ensure that any tags you've previously set up appear in the list.

blowery · 2015-11-25T15:59:57Z

Could we get screenshots, pretty please? :)

bluefuton · 2015-11-25T16:03:09Z

Could we get screenshots, pretty please? :)

Just the one :) Added to the main description.

bluefuton · 2015-12-13T22:41:01Z

Todo:

handle a missing or invalid list

We probably want to hit the single list endpoint here (/read/lists/:owner/:slug) rather than read/lists, so we can handle any 400 or 500 errors appropriately.

shaunandrews · 2015-12-14T21:41:08Z

Cool. When clicking the Manage button in the sidebar, the SectionNav defaults to Description. When I click to the Tags item, the sidebar goes back to the list selection:

The button should stay highlighted like it does in Description.

Maybe unrelated, but should we change "Feeds" to "Sites"?

…ignored (!)

Also: use smartSetState, and only display 'no results' message when last page has been fetched

bluefuton · 2015-12-15T03:11:42Z

The button should stay highlighted like it does in Description.

Yep! That should be addressed when #1535 gets merged.

should we change "Feeds" to "Sites"?

Good call. I'll change that now.

blowery · 2015-12-15T20:32:44Z

client/lib/reader-lists-tags/constants.js

+// External Dependencies
+const keyMirror = require( 'key-mirror' );
+
+module.exports.action = keyMirror( {


we're trying to avoid keyMirror now, just export const strings

blowery · 2015-12-16T20:54:36Z

client/lib/reader-lists-tags/actions.js

+import { requestInflight, requestTracker } from 'lib/inflight';
+import { action } from './constants';
+
+const TAGS_PER_PAGE = 10;


can probably jump that count up. 10 is pretty small for tag objects. Maybe 50?

…exported

blowery · 2015-12-19T01:34:50Z

client/lib/reader-lists-tags/store.js

+};
+
+function getListTags( state, listId ) {
+	return state.get( 'tags' ).get( parseInt( listId ), List() ); // eslint-disable-line new-cap


Does this make a new list instance on every call? Might want to stash the default somewhere and reuse it.

blowery · 2015-12-19T01:37:38Z

Looks good other than the one thing with possible List garbage. Ship it.

…etListTags()

Reader Lists: list followed tags

bluefuton added [Status] In Progress [Feature] Reader The reader site on Calypso. labels Nov 25, 2015

bluefuton self-assigned this Nov 25, 2015

bluefuton added this to the Reader: List Management milestone Nov 25, 2015

bluefuton force-pushed the add/reader-lists-show-contents branch from f362b9c to 94723b4 Compare December 7, 2015 23:16

bluefuton changed the title ~~Reader Lists: list followed sites and tags~~ Reader Lists: list followed tags Dec 8, 2015

bluefuton force-pushed the add/reader-lists-show-contents branch from b5a1996 to 791da6e Compare December 13, 2015 22:17

bluefuton force-pushed the add/reader-lists-show-contents branch from 6c444a3 to 931fead Compare December 14, 2015 00:03

bluefuton added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. and removed [Status] In Progress labels Dec 14, 2015

bluefuton and others added 5 commits December 15, 2015 13:55

Laid down the basics for following list display

b0af144

Fix .gitignore so that client/reader/list-management/tags dir is not …

a028875

…ignored (!)

Add tag pagination and handle fetching tags across multiple lists

7da98b3

Also: use smartSetState, and only display 'no results' message when last page has been fetched

Handle a missing or invalid list

46ce7df

Making the tag list icon smaller.

4984507

List Management: rename feeds to sites

e7c2710

bluefuton force-pushed the add/reader-lists-show-contents branch from ea175fa to e7c2710 Compare December 15, 2015 03:14

blowery reviewed Dec 15, 2015
View reviewed changes

bluefuton added [Status] Needs Author Reply and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Dec 15, 2015

bluefuton added 4 commits December 16, 2015 09:38

Address @blowery code review feedback on keyMirror() and times()

d32f38e

Correct ListManagementNavigation propTypes

7579148

Refactor ReaderListTagsStore to use createReducerStore

0f9d7e1

Use EmptyContent component when a list has no tags

d8be6c9

bluefuton added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. and removed [Status] Needs Author Reply labels Dec 16, 2015

blowery reviewed Dec 16, 2015
View reviewed changes

bluefuton added [Status] Needs Author Reply and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Dec 16, 2015

bluefuton added 5 commits December 17, 2015 08:53

Bump TAGS_PER_PAGE to 50

1f56da8

Move receiveTags() out of ReaderListsTagsStore - does not need to be …

32b27e2

…exported

ReaderListsStore: output Immutable objects only (no more toJS())

8d152ee

ReaderListsTagsStore: fix unit tests to consume Immutable objects

180e321

ReaderListsTagsStore: alter tag data structure to a Map keyed by list ID

73acb20

bluefuton added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. [Type] Task and removed [Status] Needs Author Reply labels Dec 17, 2015

bluefuton mentioned this pull request Dec 17, 2015

Reader Lists: edit title and description #1716

Merged

blowery reviewed Dec 19, 2015
View reviewed changes

bluefuton added 2 commits December 19, 2015 11:55

Store default tag list List() rather than creating on every call to g…

a3deb1e

…etListTags()

Use .size not .length on Immutable data structure, tags

27d8056

bluefuton added [Status] Ready to Merge and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Dec 19, 2015

bluefuton added a commit that referenced this pull request Dec 20, 2015

Merge pull request #777 from Automattic/add/reader-lists-show-contents

ec659e5

Reader Lists: list followed tags

bluefuton merged commit ec659e5 into master Dec 20, 2015

bluefuton deleted the add/reader-lists-show-contents branch December 20, 2015 08:32

scruffian removed the [Status] Ready to Merge label Dec 20, 2015

awaisab172 mentioned this pull request Jun 21, 2023

[Snyk] Fix for 1 vulnerabilities awaisab172/wp-calypso#152

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reader Lists: list followed tags #777

Reader Lists: list followed tags #777

bluefuton commented Nov 25, 2015

blowery commented Nov 25, 2015

bluefuton commented Nov 25, 2015

bluefuton commented Dec 13, 2015

shaunandrews commented Dec 14, 2015

bluefuton commented Dec 15, 2015

blowery Dec 15, 2015

bluefuton Dec 15, 2015

blowery Dec 16, 2015

bluefuton Dec 16, 2015

blowery Dec 19, 2015

bluefuton Dec 19, 2015

blowery commented Dec 19, 2015

Reader Lists: list followed tags #777

Reader Lists: list followed tags #777

Conversation

bluefuton commented Nov 25, 2015

To test

blowery commented Nov 25, 2015

bluefuton commented Nov 25, 2015

bluefuton commented Dec 13, 2015

shaunandrews commented Dec 14, 2015

bluefuton commented Dec 15, 2015

blowery Dec 15, 2015

Choose a reason for hiding this comment

bluefuton Dec 15, 2015

Choose a reason for hiding this comment

blowery Dec 16, 2015

Choose a reason for hiding this comment

bluefuton Dec 16, 2015

Choose a reason for hiding this comment

blowery Dec 19, 2015

Choose a reason for hiding this comment

bluefuton Dec 19, 2015

Choose a reason for hiding this comment

blowery commented Dec 19, 2015