Skip to content

Commit

Permalink
Site Selector: show placeholder only when site list is not available …
Browse files Browse the repository at this point in the history
…at all

Fixes a bug where `SiteSelector` would show a placeholder instead of list
of visible sites, even when the sites list data are available.

The `isRequestingMissingSites` selector returns `true` when there is an outstanding
request for the sites data and the local data are detected as not-fresh, i.e., the
local site count is different from the server site count, as revealed by the
`user.site_count` property.

In such a case, a placeholder is shown instead of showing the available local list.

This is a regression introduced in #13094, when `sites-list` was removed from the
component. The condition to show the placeholder used to be `! sites.initialized`,
which is much weaker than `isRequestingMissingSites`. The `sites-list` could be
initialized from local storage data and be requesting an update from the server,
without affecting the `initialized` flag.

This patch replaces the `isRequestingMissingSites` selector with `hasLoadedSites`,
which is more appropriate here.

It also fixes the `disabled` condition for the `Search` component. It used to be
`! this.props.sites`, but `this.props.sites` is always truthy, because the `getSites`
selector returns an empty object when there are no site data.

This could happen when the sites list data were detected as not-fresh
  • Loading branch information
jsnajdr committed Jan 10, 2018
1 parent cae37dd commit fae06c2
Showing 1 changed file with 4 additions and 11 deletions.
15 changes: 4 additions & 11 deletions client/components/site-selector/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,7 @@ import { getPreference } from 'state/preferences/selectors';
import { getCurrentUser } from 'state/current-user/selectors';
import { getSelectedSite } from 'state/ui/selectors';
import { getSite } from 'state/sites/selectors';
import {
areAllSitesSingleUser,
getSites,
getVisibleSites,
isRequestingMissingSites,
} from 'state/selectors';
import { areAllSitesSingleUser, getSites, getVisibleSites, hasLoadedSites } from 'state/selectors';
import AllSites from 'my-sites/all-sites';
import Site from 'blocks/site';
import SitePlaceholder from 'blocks/site/placeholder';
Expand Down Expand Up @@ -272,8 +267,7 @@ class SiteSelector extends Component {
renderSites() {
let sites;

// Assume that `sites` is falsy when loading
if ( this.props.isRequestingMissingSites ) {
if ( ! this.props.hasLoadedSites ) {
return <SitePlaceholder key="site-placeholder" />;
}

Expand Down Expand Up @@ -395,8 +389,7 @@ class SiteSelector extends Component {
onSearch={ this.onSearch }
delaySearch={ true }
autoFocus={ this.props.autoFocus }
// Assume that `sites` is falsy when loading
disabled={ ! this.props.sites }
disabled={ ! this.props.hasLoadedSites }
onSearchClose={ this.props.onClose }
onKeyDown={ this.onKeyDown }
/>
Expand Down Expand Up @@ -495,6 +488,7 @@ const mapState = state => {
const visibleSiteCount = get( user, 'visible_site_count', 0 );

return {
hasLoadedSites: hasLoadedSites( state ),
sites: getSites( state ),
showRecentSites: get( user, 'visible_site_count', 0 ) > 11,
recentSites: getPreference( state, 'recentSites' ),
Expand All @@ -503,7 +497,6 @@ const mapState = state => {
selectedSite: getSelectedSite( state ),
visibleSites: getVisibleSites( state ),
allSitesSingleUser: areAllSitesSingleUser( state ),
isRequestingMissingSites: isRequestingMissingSites( state ),
};
};

Expand Down

0 comments on commit fae06c2

Please sign in to comment.