Reader sidebar: toggle visibility of lists and tags #724

bluefuton · 2015-11-25T11:46:24Z

Rather than showing all of the user's lists and tags in the sidebar, this PR experiments with hiding them behind a toggle.

Default state:

Expanded:

cc @shaunandrews

bluefuton · 2015-11-25T11:54:18Z

As a starting point, I think I'd create a common component called something like SidebarFoldableSection (perhaps based on FoldableCard) which handles the open/close state and can be used for both tags and lists.

bluefuton · 2015-11-30T01:57:25Z

Hey @shaunandrews - in e617ccb I added a new SidebarMenu component that manages its own open/closed state, just like FoldableCard does.

That means we don't need to keep track of the open state in the sidebar itself, which should clean things up a bit. We can also use the same component to render the lists and tags menus, which should reduce duplication of logic.

The new component doesn't include the 'add new list/tag' input yet, so that'd be the next step :)

(Sidenote: I had to rebase the branch and force push to get the dependencies working, so it's worth doing a git fetch && git reset --hard origin/add/sidebar-section-toggle to grab my changes.)

shaunandrews · 2015-12-01T19:38:54Z

Awesome, @bluefuton. I've brought over the add button and input and things seem to mostly work for Lists, but adding a Tag is broken. Removing a tag is broken — not sure how to fix that. We also need to find a way to remember the state of the toggles for users.

bluefuton · 2015-12-02T21:35:50Z

Ah - I think the tags were failing because of

React.findDOMNode( this.refs.menuAddInput ).value = '';

That input is now in the MenuSidebar component, so it's not in ReaderSidebar's refs any more.

This wasn't tripping up lists because we were doing a redirect to Atlas before the line above got executed :)

I've added it to the SidebarMenu component now and it seems to be working well.

bluefuton · 2015-12-02T21:37:08Z

We also need to find a way to remember the state of the toggles for users.

Yep. If we want to remember it, we should store it in the state of the parent component, ReaderSidebar.

shaunandrews · 2016-01-11T04:21:58Z

@bluefuton I rebased this and somewhere along the way I broke the add stuff again. Mind taking another look?

bluefuton · 2016-01-11T04:26:37Z

@shaunandrews Absolutely, I'll take a look. There have been a few sidebar changes happening as part of List Management, so one of those is probably the culprit...

ralder · 2016-01-11T12:06:54Z

There are few issues what i found inside 'menu.jsx':

in function handleAddKeyDown missed input parameter event
also in this function not handled case if value of text input is empty and i press enter
since React 0.14 not need using findDOMNode
also will be good if when i press Add button text input get focus
for placeholder and button title need use translate

bluefuton · 2016-01-11T21:25:29Z

Thanks for the feedback @ralder!

I've addressed your points in 19629f7. And as a result the add input is now working again, @shaunandrews :)

bluefuton · 2016-01-14T20:57:00Z

Todo:

persist open and closed state between page views
start using Sidebar components (see Framework: add <Sidebar> component under Layout #2272)
Re-fix multiple highlight bug (regression)
Split off separate components for tags, lists and teams
Fix highlight state for tags, lists and teams

bluefuton · 2016-01-14T23:01:39Z

Big changes to the sidebar in #2272 made for a very tricky rebase 😑 All sorted now though.

bluefuton · 2016-01-19T05:32:12Z

Well, this turned into quite the 👹

...but includes a long overdue refactoring of the sidebar, and the first sprinkling of Redux in the Reader.

bluefuton · 2016-01-19T05:36:55Z

cc @blowery @shaunandrews

blowery · 2016-01-19T20:35:51Z

client/layout/sidebar/menu.jsx


-const SidebarMenu = ( { children } ) => (
-	<li className="sidebar__menu">{ children }</li>
+const SidebarMenu = ( { children, className } ) => (


hmmmm... do we really need this?

The component in general?

The class names on the component. What are we using it for? Do we need the class name?

It's the parent element inside ExpandableSidebarMenu, which passes in classes based on the toggle state:

const classes = classNames( this.props.className, { 'is-toggle-open': !! this.props.expanded, 'is-togglable': true } );

Could we set these based on props for clarity instead of passing classnames?

blowery · 2016-02-01T20:31:11Z

I think this is good to go. Whatcha think @bluefuton ? Anything left to do?

blowery · 2016-02-01T20:31:25Z

Are there new stats we need to track?

bluefuton · 2016-02-01T20:55:07Z

I think this is good to go. Whatcha think @bluefuton ? Anything left to do?

I think it'd good to get it out there, then address the "scroll the picked thing into view" and open/closed state persistence as enhancements later on.

Are there new stats we need to track?

Yep, we could potentially add one for the open and close? Let me add that & then hopefully it's 🚢 time :)

bluefuton · 2016-02-01T20:55:41Z

Add stat for open and close of expandable menu
Add stat for click of 'add' button when menu is expanded

- Adding some animations and the add button for lists and tags. - Adding transitions for the add new list/tag in the Readers sidebar and a dismiss button for the add new input. - Giving the focus to the add inputs when you click the add buttons for Tags and Lists. - Adding counts and an empty state.

toggle component.

- Clear sidebar input in SidebarMenu, not Sidebar (elements have moved) - Fix add input - Cleanup old following edit link method, and rename expandable sidebar component to avoid conflict with new SidebarMenu in layout - Rename main sidebar and ditch package.json - Use SidebarHeading component - Use sidebar__menu naming rather than sidebar-menu, to match new Sidebar components - Refactored tags section into separate components - Break out lists and teams to separate components - Moved link methods out to helper, and fixed list highlighting - Preserve expandable menu open/closed state using Redux - Style cleanup - Break out expandable menu heading to new component - When visiting a tag or list page, open the appropriate expandable menu and scroll so that the selected item is in view - Rename folders for Reader sidebar components - Only focus form input when it's first opened - fixes issue with iOS keyboard not collapsing - Reader sidebar: prevent horizontal scroll - Stats: add stat when add button is clicked for list or tag in Reader sidebar - Stats: add stat for open/close of lists and tags expandable menus

blowery · 2016-02-02T15:51:21Z

Rebased and squashed this down to author-driven commits.

Looks like we lost the new display of tag names in the move?

bluefuton · 2016-02-02T21:07:45Z

Looks like we lost the new display of tag names in the move?

Sorted in 384eaf0 👍

bluefuton · 2016-02-02T21:25:50Z

Just found an IE11 CSS bug when expanding the add input:

https://cloudup.com/c_9Gix8SUYZ

…e to

…t it. IE users will not see the placeholder.

Reader sidebar: toggle visibility of lists and tags

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

bluefuton assigned shaunandrews Nov 25, 2015

bluefuton force-pushed the add/sidebar-section-toggle branch from b4a75c0 to e617ccb Compare November 30, 2015 01:49

bluefuton force-pushed the add/sidebar-section-toggle branch from d2d8680 to 0452fb2 Compare December 2, 2015 20:48

shaunandrews force-pushed the add/sidebar-section-toggle branch from 9368db8 to c755c81 Compare January 11, 2016 04:21

bluefuton force-pushed the add/sidebar-section-toggle branch from c755c81 to 19629f7 Compare January 11, 2016 21:23

bluefuton mentioned this pull request Jan 12, 2016

Reader: reflect list title changes in sidebar #2153

Merged

shaunandrews mentioned this pull request Jan 14, 2016

Reader: Tags: change 'follow a tag' to 'find a tag' #681

Closed

bluefuton force-pushed the add/sidebar-section-toggle branch from 19629f7 to de2b33b Compare January 14, 2016 22:59

bluefuton assigned bluefuton and unassigned shaunandrews Jan 18, 2016

bluefuton added this to the Reader: List Management milestone Jan 18, 2016

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 Jan 19, 2016

bluefuton force-pushed the add/sidebar-section-toggle branch from 9a1f5ff to 6ae39ed Compare January 19, 2016 20:31

blowery reviewed Jan 19, 2016
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 Jan 28, 2016

bluefuton force-pushed the add/sidebar-section-toggle branch from bb94412 to b0f18c0 Compare February 1, 2016 22:00

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 Feb 1, 2016

shaunandrews and others added 5 commits February 2, 2016 10:42

Add SidebarMenu component that manages its own open/closed state

5964bba

Bringing the add button, add input, and a blank state to the menu

3f72d36

toggle component.

Push a stat to tracks too

d709ee9

blowery force-pushed the add/sidebar-section-toggle branch from 103c5c8 to d709ee9 Compare February 2, 2016 15:46

Use tag display_name rather than title

384eaf0

bluefuton added 3 commits February 3, 2016 10:57

Workarounds for IE add form bug and missing placeholder

663a0ab

Fix regression: handling a subscription to a tag you already subscrib…

ba1984d

…e to

Re-add autofocusing of form input - it feels an odd experience withou…

53f1d3b

…t it. IE users will not see the placeholder.

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 Feb 2, 2016

bluefuton added a commit that referenced this pull request Feb 2, 2016

Merge pull request #724 from Automattic/add/sidebar-section-toggle

d8d55b7

Reader sidebar: toggle visibility of lists and tags

bluefuton merged commit d8d55b7 into master Feb 2, 2016

bluefuton deleted the add/sidebar-section-toggle branch February 2, 2016 23:01

scruffian removed the [Status] Ready to Merge label Feb 2, 2016

bluefuton mentioned this pull request Feb 2, 2016

Reader: fix list creation redirect #3017

Merged

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 sidebar: toggle visibility of lists and tags #724

Reader sidebar: toggle visibility of lists and tags #724

bluefuton commented Nov 25, 2015

bluefuton commented Nov 25, 2015

bluefuton commented Nov 30, 2015

shaunandrews commented Dec 1, 2015

bluefuton commented Dec 2, 2015

bluefuton commented Dec 2, 2015

shaunandrews commented Jan 11, 2016

bluefuton commented Jan 11, 2016

ralder commented Jan 11, 2016

bluefuton commented Jan 11, 2016

bluefuton commented Jan 14, 2016

bluefuton commented Jan 14, 2016

bluefuton commented Jan 19, 2016

bluefuton commented Jan 19, 2016

blowery Jan 19, 2016

bluefuton Jan 19, 2016

blowery Jan 20, 2016

bluefuton Jan 21, 2016

mtias Jan 24, 2016

blowery commented Feb 1, 2016

blowery commented Feb 1, 2016

bluefuton commented Feb 1, 2016

bluefuton commented Feb 1, 2016

blowery commented Feb 2, 2016

bluefuton commented Feb 2, 2016

bluefuton commented Feb 2, 2016

Reader sidebar: toggle visibility of lists and tags #724

Reader sidebar: toggle visibility of lists and tags #724

Conversation

bluefuton commented Nov 25, 2015

bluefuton commented Nov 25, 2015

bluefuton commented Nov 30, 2015

shaunandrews commented Dec 1, 2015

bluefuton commented Dec 2, 2015

bluefuton commented Dec 2, 2015

shaunandrews commented Jan 11, 2016

bluefuton commented Jan 11, 2016

ralder commented Jan 11, 2016

bluefuton commented Jan 11, 2016

bluefuton commented Jan 14, 2016

bluefuton commented Jan 14, 2016

bluefuton commented Jan 19, 2016

bluefuton commented Jan 19, 2016

blowery Jan 19, 2016

Choose a reason for hiding this comment

bluefuton Jan 19, 2016

Choose a reason for hiding this comment

blowery Jan 20, 2016

Choose a reason for hiding this comment

bluefuton Jan 21, 2016

Choose a reason for hiding this comment

mtias Jan 24, 2016

Choose a reason for hiding this comment

blowery commented Feb 1, 2016

blowery commented Feb 1, 2016

bluefuton commented Feb 1, 2016

bluefuton commented Feb 1, 2016

blowery commented Feb 2, 2016

bluefuton commented Feb 2, 2016

bluefuton commented Feb 2, 2016