Reader: Use SectionHeading on Manage Followed Sites #1391

shaunandrews · 2015-12-08T19:18:33Z

Here's what follow management looks like on master:

The double input is really obnoxious, and I often find myself mixing up the add and search inputs. Also, the search result is a little loud.

Lets bring in the <SectionHeading> component and clean things up a bit:

We bring back the once-upon-a-time subscription count, and hide the add input behind a button. Clicking/tapping the add button transitions to show a placeholder and the focused add input:

Type a URL and you get the preview:

Hitting follow (or enter) adds a placeholder to your list, which is then replaced by the actual site:

There's also a new ellipsis icon which can be use to open a <Popover> with Import, Export, and Delivery Settings options that @blowery is working on.

@bluefuton I'd love it if you could take a look at this.

blowery · 2015-12-08T19:31:17Z

client/reader/following-edit/index.jsx

-	debounce = require( 'lodash/function/debounce' );
+	debounce = require( 'lodash/function/debounce' ),
+	classnames = require( 'classnames' ),
+	assign = require( 'lodash/object/assign' );


Can use Object.assign directly now, instead of pulling in lodash.

bluefuton · 2015-12-08T21:13:33Z

This is a vast improvement over the double-input-box UI. Kudos @shaunandrews!

One small thing: I noticed that the sidebar doesn't seem to collapse away correctly and I can see part of the nav underneath the Manage Following content at smaller window sizes. This also means the MobileBackToSidebar link doesn't work as expected:

bluefuton · 2015-12-09T22:01:04Z

Here's a screen recording of the odd sidebar behaviour: https://cloudup.com/iyMatVc96R3

rickybanister · 2015-12-09T22:02:17Z

I think this is much better.

It offers improved context (seeing the total number of sites), and a central place for all the related tools to preside over the interface. The search makes sense being attached to the list in this case.

I was worried that we're switching the order of the SectionNav (search) and the SectionHeader (toolbar) but the logic behind it in this case makes more sense.

The only note on the code will go away once #1358 gets merged (Search will use gridicons)

mtias · 2015-12-10T14:45:58Z

client/reader/following-edit/index.jsx

+
+					<FollowingEditSortControls onSelectChange={ this.handleSortOrderChange } sortOrder={ this.state.sortOrder } />
+
+					<SectionHeaderButton onClick={ this.toggleAddSite }>


SectionHeader button is deprecated, just using a regular Button inside it with the compact prop should be the same.

component.

the search results and adding a blank slate.

gwwar · 2015-12-16T19:35:07Z

client/components/search/index.jsx

 	},

 	getInitialState: function() {
 		return {
-			keyword: this.props.initialValue || ''
+			keyword: this.props.initialValue || '',
+			isOpen: this.props.isOpen || false


Not sure if we prefer it, but isOpen: !! this.props.isOpen works as well.

👍 Updated in 18b7b1d.

gwwar · 2015-12-16T19:53:04Z

This is a great improvement. 👍 The double inputs were bothering me too. Just a few small things:

Maybe a smidgen of padding here:

I feel like the top header could use a bit of work. The add button could stand out a bit more (I had to hunt for it) and what it does is a bit cryptic. As a user I wouldn't expect it to transform the mode. Not sure if the count pill is very useful here either. I kept expecting it to update with the search results, but it's static, so maybe some more descriptive text there.

I think it would be pretty interesting to get some users to test this, and see if they run into any trouble.

rralian · 2015-12-16T21:50:51Z

client/reader/following-edit/sort-controls.jsx

@@ -31,11 +31,11 @@ const FollowingEditSortControls = React.createClass( {
 		const sortOrder = this.props.sortOrder;

 		return (
-			<div className="following-edit-sort-controls">
+			<div className="following-edit__sort-controls">
 				<label htmlFor="sort-control-select">{ this.translate( 'Sort by' ) }</label>
 				<select id="sort-control-select" ref="sortControlSelect" className="is-compact" onChange={ this.handleSelectChange } value={ sortOrder }>


Probably not necessary in this PR, but could we switch to using select dropdown instead of a select element here for consistency?

Or maybe segmented-control?

rickybanister · 2015-12-16T21:51:15Z

This placement and style of Add button is becoming a pattern across calypso, it just hasn't been put in place anywhere yet. Plugins and People will be the next sections to implement it, but I don't see any problem with its use here.

In the plans for those other sections we've been playing with the idea of an icon in conjunction with the button text. Also perhaps in this case it could stand to be a primary button, simply making it blue to stand out.

The count next to the SectionHeader title is another standard we're using elsewhere. It counts the number of sites you follow, not the number of sites returned by autocomplete.

rralian · 2015-12-16T22:05:22Z

The add button [...] is is a bit cryptic.

I would agree with that. I think the dropdown is a bit terse too. So while this is visually cleaner than what's in master, it doesn't seem as functionally clear to me. We have quite a bit of room though... could we be a bit more explicit with the "add" and "by date/by name" controls?

shaunandrews · 2015-12-17T21:50:01Z

I made the add button a little more specific (Follow Site) and made it primary (blue):

We have quite a bit of room though...

We have room on desktop (though, I don't think that in itself is a good reason to add more stuff), but on mobile we're pretty close to being "full":

bluefuton · 2015-12-17T22:10:43Z

I made the add button a little more specific (Follow Site) and made it primary (blue)

👍 Looks great @shaunandrews.

I think I prefer to 'Add Site' to 'Follow Site' as a call to action, but both work.

rickybanister · 2015-12-17T22:19:04Z

I'm with @bluefuton, but it looks good Primary.

shaunandrews · 2015-12-18T14:43:44Z

The thinking behind "Follow Site" over "Add Site" was to hopefully quell any confusion between creating a new site and adding a site to your followed list. When viewing a site or post in Reader (or the front end) the button is "Follow" not "Add" — so I was thinking the consistency would help.

folletto · 2015-12-18T14:58:18Z

👍

…header Reader: Use SectionHeading on Manage Followed Sites

shaunandrews added [Status] In Progress [Feature] Reader The reader site on Calypso. labels Dec 8, 2015

shaunandrews self-assigned this Dec 8, 2015

blowery reviewed Dec 8, 2015
View reviewed changes

rickybanister mentioned this pull request Dec 8, 2015

Reader: Add inbox view #351

Closed

5 tasks

shaunandrews 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 9, 2015

mtias reviewed Dec 10, 2015
View reviewed changes

mtias 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 10, 2015

rickybanister mentioned this pull request Dec 10, 2015

Framework: removed last remaining instance of SectionHeaderButton #1468

Merged

shaunandrews added 10 commits December 15, 2015 09:58

Fixing a typo in the section heading styles.

9afa797

Adding the SectionHeading component for edit following.

e32724f

Adding the sorter to the new SectionHeader

07cf462

Adding a prop to force a close button to always show in the Search

95d9f5b

component.

Tweaking the sorter’s labels.

320682f

Making the close button always show on the add site input.

86a58ba

Adding a state to toggle the add input.

0b7f743

Adding focus to the add input when the button is clicked and updating

02ed96e

the search results and adding a blank slate.

Making a few tweaks for mobile sizes.

15d53f5

Adding the subscription count.

2063651

shaunandrews and others added 2 commits December 15, 2015 10:01

The add input doesn’t need autoFocus or isOpen.

8531782

Fix styling issues after rebase

63db848

bluefuton force-pushed the update/reader-follow-manage-header branch from 0abfc02 to 63db848 Compare December 15, 2015 00:59

bluefuton added 2 commits December 15, 2015 12:24

Remove ellipsis from SectionHeader until it's needed

15fe779

Bring back forceCloseButton prop for Search

31e8018

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 15, 2015

shaunandrews added 4 commits December 15, 2015 11:21

Fixing the transition on the search-card, rather than just hiding it.

9c2affc

Adding a rotation to the entrance transition on the add gridicon.

f25eadb

Removing the forceCloseButton prop and adding an isOpen prop.

d207550

Using the isOpen prop instead of the forceCloseButton prop.

ad6c069

gwwar reviewed Dec 16, 2015
View reviewed changes

rralian reviewed Dec 16, 2015
View reviewed changes

bluefuton and others added 2 commits December 17, 2015 13:52

Use imports for dependencies in following-edit/index

18b7b1d

Updating the add button to say “Add Site” and be a primary button.

4744517

shaunandrews 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 18, 2015

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

Merge pull request #1391 from Automattic/update/reader-follow-manage-…

bd5162f

…header Reader: Use SectionHeading on Manage Followed Sites

bluefuton merged commit bd5162f into master Dec 18, 2015

bluefuton deleted the update/reader-follow-manage-header branch December 18, 2015 23:28

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reader: Use SectionHeading on Manage Followed Sites #1391

Reader: Use SectionHeading on Manage Followed Sites #1391

shaunandrews commented Dec 8, 2015

blowery Dec 8, 2015

blowery Dec 8, 2015

bluefuton commented Dec 8, 2015

bluefuton commented Dec 9, 2015

rickybanister commented Dec 9, 2015

mtias Dec 10, 2015

gwwar Dec 16, 2015

bluefuton Dec 17, 2015

gwwar commented Dec 16, 2015

rralian Dec 16, 2015

rralian Dec 16, 2015

rickybanister commented Dec 16, 2015

rralian commented Dec 16, 2015

shaunandrews commented Dec 17, 2015

bluefuton commented Dec 17, 2015

rickybanister commented Dec 17, 2015

shaunandrews commented Dec 18, 2015

folletto commented Dec 18, 2015


		<FollowingEditSortControls onSelectChange={ this.handleSortOrderChange } sortOrder={ this.state.sortOrder } />

		<SectionHeaderButton onClick={ this.toggleAddSite }>

Reader: Use SectionHeading on Manage Followed Sites #1391

Reader: Use SectionHeading on Manage Followed Sites #1391

Conversation

shaunandrews commented Dec 8, 2015

blowery Dec 8, 2015

Choose a reason for hiding this comment

blowery Dec 8, 2015

Choose a reason for hiding this comment

bluefuton commented Dec 8, 2015

bluefuton commented Dec 9, 2015

rickybanister commented Dec 9, 2015

mtias Dec 10, 2015

Choose a reason for hiding this comment

gwwar Dec 16, 2015

Choose a reason for hiding this comment

bluefuton Dec 17, 2015

Choose a reason for hiding this comment

gwwar commented Dec 16, 2015

rralian Dec 16, 2015

Choose a reason for hiding this comment

rralian Dec 16, 2015

Choose a reason for hiding this comment

rickybanister commented Dec 16, 2015

rralian commented Dec 16, 2015

shaunandrews commented Dec 17, 2015

bluefuton commented Dec 17, 2015

rickybanister commented Dec 17, 2015

shaunandrews commented Dec 18, 2015

folletto commented Dec 18, 2015