Admin Page: display Sharing as new default tab for non-admins

This commit changes the behaviour of the Settings screen for connected non-admins.

Non-admins currently need access to the Settings screen for 2 reasons:
- Publicize (Sharing tab)
- Post By Email (Writing tab)

Until now, we always defaulted to the Writing tab for connected non-admins. This has 2 problems:
- We did not check if the Post By Email module was actually enabled, so when the module was disabled editors loading the settings page would only see a Writing tab and its title, but no content.
- Publicize is more popular than Post By Email today, so it's more likely that editors will be interested in the Publicize settings first, even if both PBE and Publicize are enabled.

This commit changes that default behaviour, and introduces checks for the active modules to avoid displaying tabs when we do not need to.

Previous work on this: #13541

projects/plugins/jetpack/_inc/client/components/navigation-settings/index.jsx

@@ -106,7 +106,7 @@ export const NavigationSettings = createReactClass( {
 	},
 
 	render: function () {
-		let navItems, sharingTab;
+		let navItems, sharingTab, writingTab;
 		if ( this.props.userCanManageModules ) {
 			navItems = (
 				<NavTabs selectedText={ this.props.routeName }>
@@ -192,37 +192,42 @@ export const NavigationSettings = createReactClass( {
 		} else if ( this.props.isSubscriber ) {
 			navItems = false;
 		} else {
+			// Show a sharing tab if the Publicize module is active and the user can publish.
 			if ( ! this.props.isModuleActivated( 'publicize' ) || ! this.props.userCanPublish ) {
 				sharingTab = '';
 			} else {
 				sharingTab = this.props.hasAnyOfTheseModules( [ 'publicize' ] ) && (
 					<NavItem
 						path="#sharing"
 						onClick={ this.handleClickForTracking( 'sharing' ) }
-						selected={ this.props.location.pathname === '/sharing' }
+						selected={
+							this.props.location.pathname === '/sharing' ||
+							this.props.location.pathname === '/settings'
+						}
 					>
 						{ _x( 'Sharing', 'Navigation item.', 'jetpack' ) }
 					</NavItem>
 				);
 			}
+
+			// Show a Writing tab if the Post By Email module is active and the user can publish.
+			if ( ! this.props.isModuleActivated( 'post-by-email' ) || ! this.props.userCanPublish ) {
+				writingTab = '';
+			} else {
+				writingTab = this.props.hasAnyOfTheseModules( [ 'post-by-email' ] ) && (
+					<NavItem
+						path="#writing"
+						onClick={ this.handleClickForTracking( 'writing' ) }
+						selected={ this.props.location.pathname === '/writing' }
+					>
+						{ _x( 'Writing', 'Navigation item.', 'jetpack' ) }
+					</NavItem>
+				);
+			}
 			navItems = (
 				<NavTabs selectedText={ this.props.routeName }>
-					{ this.props.hasAnyOfTheseModules( [ 'post-by-email' ] ) && (
-						<NavItem
-							path="#writing"
-							onClick={ this.handleClickForTracking( 'writing' ) }
-							selected={
-								this.props.location.pathname === '/writing' ||
-								this.props.location.pathname === '/settings'
-							}
-						>
-							{ _x( 'Writing', 'Navigation item.', 'jetpack' ) }
-						</NavItem>
-					) }
-					{
-						// Give only Publicize to non admin users
-						sharingTab
-					}
+					{ writingTab }
+					{ sharingTab }
 				</NavTabs>
 			);
 		}

projects/plugins/jetpack/_inc/client/settings/index.jsx

@@ -1,11 +1,10 @@
 /**
  * External dependencies
- *
- * @format
  */
-
 import React from 'react';
 import { __, sprintf } from '@wordpress/i18n';
+import { connect } from 'react-redux';
+import { withRouter } from 'react-router-dom';
 
 /**
  * Internal dependencies
@@ -18,71 +17,87 @@ import Security from 'security';
 import Sharing from 'sharing';
 import Traffic from 'traffic';
 import Writing from 'writing';
-import { withRouter } from 'react-router-dom';
+import { isModuleActivated as isModuleActivatedSelector } from 'state/modules';
 
 class Settings extends React.Component {
 	static displayName = 'SearchableSettings';
 
 	render() {
+		const {
+			location = { pathname: '' },
+			rewindStatus,
+			searchTerm,
+			siteAdminUrl,
+			siteRawUrl,
+			userCanManageModules,
+		} = this.props;
+		const { pathname } = location;
 		const commonProps = {
-			searchTerm: this.props.searchTerm,
-			rewindStatus: this.props.rewindStatus,
-			userCanManageModules: this.props.userCanManageModules,
+			searchTerm,
+			rewindStatus,
+			userCanManageModules,
 		};
 
 		return (
 			<div className="jp-settings-container">
 				<div className="jp-no-results">
-					{ commonProps.searchTerm
+					{ searchTerm
 						? sprintf(
 								/* translators: placeholder is a searchterm entered in searchform. */
 								__( 'No search results found for %s', 'jetpack' ),
-								commonProps.searchTerm
+								searchTerm
 						  )
 						: __( 'Enter a search term to find settings or close search.', 'jetpack' ) }
 				</div>
 				<Security
-					siteAdminUrl={ this.props.siteAdminUrl }
-					siteRawUrl={ this.props.siteRawUrl }
+					siteAdminUrl={ siteAdminUrl }
+					siteRawUrl={ siteRawUrl }
 					active={
-						'/security' === this.props.location.pathname ||
-						( '/settings' === this.props.location.pathname && commonProps.userCanManageModules )
+						'/security' === pathname || ( '/settings' === pathname && userCanManageModules )
 					}
 					{ ...commonProps }
 				/>
 				<Discussion
-					siteRawUrl={ this.props.siteRawUrl }
-					active={ '/discussion' === this.props.location.pathname }
-					{ ...commonProps }
-				/>
-				<Performance
-					active={ '/performance' === this.props.location.pathname }
+					siteRawUrl={ siteRawUrl }
+					active={ '/discussion' === pathname }
 					{ ...commonProps }
 				/>
+				<Performance active={ '/performance' === pathname } { ...commonProps } />
 				<Traffic
-					siteRawUrl={ this.props.siteRawUrl }
-					siteAdminUrl={ this.props.siteAdminUrl }
-					active={ '/traffic' === this.props.location.pathname }
+					siteRawUrl={ siteRawUrl }
+					siteAdminUrl={ siteAdminUrl }
+					active={ '/traffic' === pathname }
 					{ ...commonProps }
 				/>
 				<Writing
-					siteAdminUrl={ this.props.siteAdminUrl }
+					siteAdminUrl={ siteAdminUrl }
 					active={
-						'/writing' === this.props.location.pathname ||
-						( '/settings' === this.props.location.pathname && ! commonProps.userCanManageModules )
+						'/writing' === pathname ||
+						( ! userCanManageModules &&
+							this.props.isModuleActivated( 'post-by-email' ) &&
+							! this.props.isModuleActivated( 'publicize' ) )
 					}
 					{ ...commonProps }
 				/>
 				<Sharing
-					siteAdminUrl={ this.props.siteAdminUrl }
-					active={ '/sharing' === this.props.location.pathname }
+					siteAdminUrl={ siteAdminUrl }
+					active={
+						'/sharing' === pathname ||
+						( '/settings' === pathname &&
+							! userCanManageModules &&
+							this.props.isModuleActivated( 'publicize' ) )
+					}
 					{ ...commonProps }
 				/>
-				<Privacy active={ '/privacy' === this.props.location.pathname } { ...commonProps } />
-				<SearchableModules searchTerm={ this.props.searchTerm } />
+				<Privacy active={ '/privacy' === pathname } { ...commonProps } />
+				<SearchableModules searchTerm={ searchTerm } />
 			</div>
 		);
 	}
 }
 
-export default withRouter( Settings );
+export default connect( state => {
+	return {
+		isModuleActivated: module => isModuleActivatedSelector( state, module ),
+	};
+} )( withRouter( Settings ) );