Framework: Create new <SitesDropdown> component. #379

mtias · 2015-11-21T11:01:52Z

This should replace all usages of "me/select-site" and standardize all site selection with our core picker patterns.

To-do

Highlight selected site in list.
Handle onSelect actions.
Testing.

artpi · 2015-11-25T09:50:04Z

I know this is in progress, so just dropping it here:

Uncaught ReferenceError: postsBase is not defined

While going to http://calypso.localhost:3000/stats/day/[site]

mtias · 2015-12-09T12:10:12Z

Added example in devdocs for testing:

artpi · 2015-12-09T13:18:59Z

Ok, seems good :)
🐑 and 🚢

folletto · 2015-12-09T23:34:37Z

Tested on Chrome, Firefox, Safari, Edge, IE11.

Notes:

When there are multiple items (more than available space), we need to make sure that the last item is cut in half to hint that scrolling is possible. I missed this bit in the mockups.
The dropdown seems pushing the content down instead of being "over" the content. Is that just a side effect in the documentation page?
The chevron seems misaligned: with top: 21px seems v-centering it.
On IE11 it's showing the update icon, overlapping the chevron (screenshot).
I found odd when searching that the item already selected didn't show up. However I'm not sure here. Just raising the point. I think I expected it to show.
Can we animate the chevron? transition: transform 0.15s cubic-bezier(0.175, 0.885, 0.32, 1.275), color 0.2s ease-in;

That's all. Interaction wise (open / close / site display / search) works.

mtias · 2015-12-10T14:17:57Z

@folletto thanks for the feedback!

When there are multiple items (more than available space), we need to make sure that the last item is cut in half to hint that scrolling is possible. I missed this bit in the mockups.

I don't think we need to do this right now. It's using vh units so you can make the most use of the screen.

The dropdown seems pushing the content down instead of being "over" the content. Is that just a side effect in the documentation page?

Made it overflow in 7ad7e49

On IE11 it's showing the update icon, overlapping the chevron (screenshot).

That's not just IE, it's everywhere, will disable the indicator for it :)

I found odd when searching that the item already selected didn't show up. However I'm not sure here. Just raising the point. I think I expected it to show.

I think it's fine and consistent — since search applies to what's below, and your selected site is at the top.

Can we animate the chevron? transition: transform 0.15s cubic-bezier(0.175, 0.885, 0.32, 1.275), color 0.2s ease-in;

Done. Though the animation seems a bit frantic to me.

folletto · 2015-12-10T17:51:06Z

I think it's fine and consistent — since search applies to what's below, and your selected site is at the top.

Thing is, I expect the site to be below too. ;)

Dropdowns don't remove the element that's picked. It's a standard behaviour. I just noticed now that the selected one isn't in the list. Should be.

Ok. Tested in Chrome / Firefox, all the above are fixed. Just... a tiny z-index matter:

mtias · 2015-12-10T18:23:33Z

Thing is, I expect the site to be below too. ;)

That's v2!

Dropdowns don't remove the element that's picked. It's a standard behaviour. I just noticed now that the selected one isn't in the list. Should be.

That's what we talked about 😧 " It's probably superfluous given it's also at the top, eh?" so now the selected site is hidden from the list.

folletto · 2015-12-10T18:33:25Z

I thought we were talking about the highlight of the currently selected
item (gray background) not removing it entirely :)

Davide ‘Folletto’ Casali
AUTOMATTIC

seear · 2015-12-11T12:29:42Z

The old /me/select-site component allows a filtered list of sites to be specified, which /design uses to filter out Jetpack sites. That could be added in #1483.

seear · 2015-12-11T12:38:29Z

Code looks good 👍

I checked out the devdocs example before reading any of the PR discussion and did think for a moment that it was using a cutdown list of only 3 sites:

ockham · 2015-12-11T13:52:12Z

As a consumer of this component, how can I find out which site was actually selected?

ockham · 2015-12-11T13:57:21Z

(I'm asking because I can't seem to see an easy way. Maybe selectSite() should accept a prop function to which the args of SiteSelector#onSiteSelect, i.e. siteSlug and event are passed?)

mtias · 2015-12-11T15:39:23Z

@ockham we just need to provide a props.onSiteSelect here https://github.com/Automattic/wp-calypso/pull/379/files#diff-5e6a6bd4c81e9dfa2ddb58671cbd501eR54

mtias · 2015-12-11T15:39:56Z

What you said :)

ockham · 2015-12-11T17:22:31Z

Pushed 71451cf to #1483 -- feel free to cherry-pick :-)

ockham · 2015-12-11T17:56:55Z

client/components/sites-dropdown/index.jsx

+	},
+
+	getSelectedSite() {
+		return sites.getSite( this.state.selected ) || sites.getPrimary();


Do we need the || sites.getPrimary() part? Since getInitialState() sets this.state.selected and falls back to sites.getPrimary().slug, we should be fine without, no?

(This is going to be more relevant once we add filtering.)

Yes, I think we should be fine without it.

This should replace all usages of "me/select-site" and standardize all site selection with our core picker patterns.

Pass site slug onSiteSelect as the prop callback.

- Move all basic styles to site-selector. - Move all the sidebar specific styling under the sidebar namespace. - Clean up resets in sites-popover.

- Add a "hideSelected" prop that allows hiding the selected site from the list. - If not set, the site is shown and highlighted.

mtias · 2015-12-14T11:48:36Z

@ockham thanks for the review. How is it looking now? I think we can merge and add the filtering later on when you actually need to use it in #1483.

ockham · 2015-12-14T13:17:26Z

👍

mtias · 2015-12-14T13:36:20Z

Will squash a couple commits.

Animate chevron icon when opening. Disable site-indicator in dropdown header.

@ockham

props @ockham. Avoid redundand getPrimary() call.

Framework: Create new <SitesDropdown> component.

gwwar · 2015-12-14T17:20:53Z

assets/stylesheets/layout/_sidebar.scss

+		left: -272px;
+	width: 272px;
+	overflow: hidden;
+	z-index: 10;


Could you add this to z-index map?

Ah, good call.

mtias added Site Picker Framework [Status] In Progress Components labels Nov 21, 2015

mtias mentioned this pull request Nov 22, 2015

Me / Site Picker: suggestion: use blog picker styles for primary blog dropdown #451

Closed

rachelmcr mentioned this pull request Nov 23, 2015

Help: Replace the old site picker with the new one being developed #520

Closed

artpi assigned artpi and unassigned artpi Nov 24, 2015

mtias force-pushed the add/sites-dropdown branch from 6c88097 to 44b2950 Compare December 9, 2015 10:46

mtias 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 force-pushed the add/sites-dropdown branch from 597407b to db79348 Compare December 9, 2015 13:10

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

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

ockham mentioned this pull request Dec 10, 2015

SiteSelectorModal: Use SitesDropdown #1483

Merged

ockham reviewed Dec 11, 2015
View reviewed changes

mtias added 8 commits December 14, 2015 12:40

Create new <SitesDropdown> component.

d6d84de

This should replace all usages of "me/select-site" and standardize all site selection with our core picker patterns.

Update SiteSelector to allow non link actions on selection.

0440da8

Pass site slug onSiteSelect as the prop callback.

Site dropdown: change selected site on selection action.

f713ade

Site Selector: significant styles refactor.

53d9646

- Move all basic styles to site-selector. - Move all the sidebar specific styling under the sidebar namespace. - Clean up resets in sites-popover.

Sites: allow not showing the current selected site.

57cf9fa

- Add a "hideSelected" prop that allows hiding the selected site from the list. - If not set, the site is shown and highlighted.

Add "sites-dropdown" readme file.

53a8a73

Devdocs: add example usage of <SitesDropdown>.

5be7f09

onSiteSelect should noop by default.

bc46a72

mtias force-pushed the add/sites-dropdown branch from d836efe to 8d196a0 Compare December 14, 2015 11:41

ockham 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 14, 2015

mtias added 2 commits December 14, 2015 14:37

Make dropdown open overflowing content below.

e087e15

Animate chevron icon when opening. Disable site-indicator in dropdown header.

SitesDropdown: support onSiteSelect prop.

888e7f6

props @ockham. Avoid redundand getPrimary() call.

mtias force-pushed the add/sites-dropdown branch from 398ade4 to 888e7f6 Compare December 14, 2015 13:37

mtias added a commit that referenced this pull request Dec 14, 2015

Merge pull request #379 from Automattic/add/sites-dropdown

cb701c2

Framework: Create new <SitesDropdown> component.

mtias merged commit cb701c2 into master Dec 14, 2015

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

mtias deleted the add/sites-dropdown branch December 14, 2015 13:47

gwwar reviewed Dec 14, 2015
View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Framework: Create new <SitesDropdown> component. #379

Framework: Create new <SitesDropdown> component. #379

mtias commented Nov 21, 2015

artpi commented Nov 25, 2015

mtias commented Dec 9, 2015

artpi commented Dec 9, 2015

folletto commented Dec 9, 2015

mtias commented Dec 10, 2015

folletto commented Dec 10, 2015

mtias commented Dec 10, 2015

folletto commented Dec 10, 2015

seear commented Dec 11, 2015

seear commented Dec 11, 2015

ockham commented Dec 11, 2015

ockham commented Dec 11, 2015

mtias commented Dec 11, 2015

mtias commented Dec 11, 2015

ockham commented Dec 11, 2015

ockham Dec 11, 2015

mtias Dec 14, 2015

mtias commented Dec 14, 2015

ockham commented Dec 14, 2015

mtias commented Dec 14, 2015

gwwar Dec 14, 2015

mtias Dec 14, 2015

Framework: Create new <SitesDropdown> component. #379

Framework: Create new <SitesDropdown> component. #379

Conversation

mtias commented Nov 21, 2015

To-do

artpi commented Nov 25, 2015

mtias commented Dec 9, 2015

artpi commented Dec 9, 2015

folletto commented Dec 9, 2015

mtias commented Dec 10, 2015

folletto commented Dec 10, 2015

mtias commented Dec 10, 2015

folletto commented Dec 10, 2015

seear commented Dec 11, 2015

seear commented Dec 11, 2015

ockham commented Dec 11, 2015

ockham commented Dec 11, 2015

mtias commented Dec 11, 2015

mtias commented Dec 11, 2015

ockham commented Dec 11, 2015

ockham Dec 11, 2015

Choose a reason for hiding this comment

mtias Dec 14, 2015

Choose a reason for hiding this comment

mtias commented Dec 14, 2015

ockham commented Dec 14, 2015

mtias commented Dec 14, 2015

gwwar Dec 14, 2015

Choose a reason for hiding this comment

mtias Dec 14, 2015

Choose a reason for hiding this comment