Skip to content

Commit

Permalink
Add: App Banner Visibility state - hide banner in sidebar and post er…
Browse files Browse the repository at this point in the history
…ror views (#56772)

* AppBannerVisibility: Add UI Selectors, reducers and actions

* AppBannerVisibility: Add to the App Banner Component

* AppBannerVisibility: Update reader Sidebar

Hide the app banner when the sidebar is open

* AppBannerVisibility: Update the single post App banner visibility do not show the App Banner if the post is_error

* Update to use state and new names

* Use getCurrentLayoutFocus selector

* Try to refresh the banner state if we have post state has changed.

* Only run the call if the post is loaded.

* Fixup the app banner update logic

Co-authored-by: Jarda Snajdr <[email protected]>
  • Loading branch information
enejb and jsnajdr authored Oct 15, 2021
1 parent 1cce66a commit 8b7daf5
Show file tree
Hide file tree
Showing 9 changed files with 66 additions and 2 deletions.
10 changes: 8 additions & 2 deletions client/blocks/app-banner/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import { getPreference, isFetchingPreferences } from 'calypso/state/preferences/
import getCurrentRoute from 'calypso/state/selectors/get-current-route';
import isNotificationsOpen from 'calypso/state/selectors/is-notifications-open';
import { shouldDisplayTosUpdateBanner } from 'calypso/state/selectors/should-display-tos-update-banner';
import { getSectionName, getSelectedSiteId } from 'calypso/state/ui/selectors';
import { getSectionName, getSelectedSiteId, appBannerIsEnabled } from 'calypso/state/ui/selectors';
import {
ALLOWED_SECTIONS,
EDITOR,
Expand Down Expand Up @@ -77,7 +77,7 @@ export class AppBanner extends Component {
};

isVisible() {
const { dismissedUntil, currentSection, isTosBannerVisible } = this.props;
const { dismissedUntil, currentSection, isTosBannerVisible, isAppBannerEnabled } = this.props;

// The ToS update banner is displayed in the same position as the mobile app banner. Since the ToS update
// has higher priority, we repress all other non-essential sticky banners if the ToS update banner needs to
Expand All @@ -86,6 +86,11 @@ export class AppBanner extends Component {
return false;
}

// In some cases such as error we want to hide the app banner completely.
if ( ! isAppBannerEnabled ) {
return false;
}

return this.isMobile() && ! isWpMobileApp() && ! isDismissed( dismissedUntil, currentSection );
}

Expand Down Expand Up @@ -243,6 +248,7 @@ const mapStateToProps = ( state ) => {
fetchingPreferences: isFetchingPreferences( state ),
siteId: getSelectedSiteId( state ),
isTosBannerVisible: shouldDisplayTosUpdateBanner( state ),
isAppBannerEnabled: appBannerIsEnabled( state ),
};
};

Expand Down
24 changes: 24 additions & 0 deletions client/blocks/reader-full-post/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ import getCurrentStream from 'calypso/state/selectors/get-reader-current-stream'
import isFeedWPForTeams from 'calypso/state/selectors/is-feed-wpforteams';
import isSiteWPForTeams from 'calypso/state/selectors/is-site-wpforteams';
import { getReaderTeams } from 'calypso/state/teams/selectors';
import { disableAppBanner, enableAppBanner } from 'calypso/state/ui/actions';
import ReaderFullPostHeader from './header';
import ReaderFullPostContentPlaceholder from './placeholders/content';
import ReaderFullPostUnavailable from './unavailable';
Expand Down Expand Up @@ -108,6 +109,7 @@ export class FullPostView extends Component {
this.hasSentPageView = false;
this.hasLoaded = false;
this.attemptToSendPageView();
this.maybeDisableAppBanner();

this.checkForCommentAnchor();

Expand All @@ -131,6 +133,7 @@ export class FullPostView extends Component {
this.hasSentPageView = false;
this.hasLoaded = false;
this.attemptToSendPageView();
this.maybeDisableAppBanner();
}

if ( this.props.shouldShowComments && ! prevProps.shouldShowComments ) {
Expand All @@ -143,6 +146,13 @@ export class FullPostView extends Component {
if ( this.hasCommentAnchor && ! this.hasScrolledToCommentAnchor ) {
this.scrollToComments();
}

// Check if we just finished loading the post and enable the app banner when there's no error
const finishedLoading = prevProps.post?._state === 'pending' && ! this.props.post?._state;
const isError = this.props.post?.is_error;
if ( finishedLoading && ! isError ) {
this.props.enableAppBanner();
}
}

componentWillUnmount() {
Expand All @@ -153,6 +163,7 @@ export class FullPostView extends Component {
KeyboardShortcuts.off( 'move-selection-up', this.goToPreviousPost );
// Remove WPiFrameResize listener.
this.stopResize?.();
this.props.enableAppBanner(); // reset the app banner
}

handleBack = ( event ) => {
Expand Down Expand Up @@ -297,6 +308,17 @@ export class FullPostView extends Component {
}
};

maybeDisableAppBanner = () => {
const { post, site } = this.props;

// disable the banner while the post is loading and when it failed to load
const isLoading = post?._state === 'pending';
const isError = post?.is_error || site?.is_error;
if ( isLoading || isError ) {
this.props.disableAppBanner();
}
};

goToNextPost = () => {
if ( this.props.nextPost ) {
showSelectedPost( { postKey: this.props.nextPost } );
Expand Down Expand Up @@ -624,6 +646,8 @@ export default connect(
return props;
},
{
disableAppBanner,
enableAppBanner,
markPostSeen,
setViewingFullPostKey,
unsetViewingFullPostKey,
Expand Down
1 change: 1 addition & 0 deletions client/state/action-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -542,6 +542,7 @@ export const MARKETPLACE_RESET_PURCHASE_FLOW = 'MARKETPLACE_RESET_PURCHASE_FLOW'
export const MARKETPLACE_SITE_TRANSFER_STATE_CHANGE = 'MARKETPLACE_SITE_TRANSFER_STATE_CHANGE';
export const MARKETPLACE_SITE_TRANSFER_PLUGIN_INSTALL = 'MARKETPLACE_SITE_TRANSFER_PLUGIN_INSTALL';
export const MASTERBAR_TOGGLE_VISIBILITY = 'MASTERBAR_TOGGLE_VISIBILITY';
export const APP_BANNER_TOGGLE_VISIBILITY = 'APP_BANNER_TOGGLE_VISIBILITY';
export const MEDIA_CLEAR_SITE = 'MEDIA_CLEAR_SITE';
export const MEDIA_DELETE = 'MEDIA_DELETE';
export const MEDIA_ERRORS_CLEAR = 'MEDIA_ERRORS_CLEAR';
Expand Down
1 change: 1 addition & 0 deletions client/state/ui/actions/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,6 @@
*/
export { setSection, setSectionLoading } from '../section/actions';
export { setSelectedSiteId, setAllSitesSelected } from './set-sites';
export { disableAppBanner, enableAppBanner } from '../app-banner-visibility/actions';
export { showMasterbar, hideMasterbar } from '../masterbar-visibility/actions';
export { toggleNotificationsPanel } from './notifications';
17 changes: 17 additions & 0 deletions client/state/ui/app-banner-visibility/actions.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import { APP_BANNER_TOGGLE_VISIBILITY } from 'calypso/state/action-types';

import 'calypso/state/ui/init';

/**
* Hide the AppBanner.
*
* @returns {object} Action object
*/
export const disableAppBanner = () => ( { type: APP_BANNER_TOGGLE_VISIBILITY, isVisible: false } );

/**
* Show the AppBanner.
*
* @returns {object} Action object
*/
export const enableAppBanner = () => ( { type: APP_BANNER_TOGGLE_VISIBILITY, isVisible: true } );
4 changes: 4 additions & 0 deletions client/state/ui/app-banner-visibility/reducer.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
import { APP_BANNER_TOGGLE_VISIBILITY } from 'calypso/state/action-types';

export default ( state = true, { type, isVisible } ) =>
type === APP_BANNER_TOGGLE_VISIBILITY ? isVisible : state;
2 changes: 2 additions & 0 deletions client/state/ui/reducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
} from 'calypso/state/action-types';
import { combineReducers, withSchemaValidation } from 'calypso/state/utils';
import actionLog from './action-log/reducer';
import appBannerVisibility from './app-banner-visibility/reducer';
import checkout from './checkout/reducer';
import language from './language/reducer';
import layoutFocus from './layout-focus/reducer';
Expand Down Expand Up @@ -71,6 +72,7 @@ export function isNotificationsOpen( state = false, { type } ) {

const reducer = combineReducers( {
actionLog,
appBannerVisibility,
checkout,
isSectionLoading,
isNotificationsOpen,
Expand Down
8 changes: 8 additions & 0 deletions client/state/ui/selectors/app-banner-is-visible.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import 'calypso/state/ui/init';
import { getCurrentLayoutFocus } from 'calypso/state/ui/layout-focus/selectors';

export default function isAppBannerEnabled( state ) {
// since on mobile the sidebar layout focus take up the whole "Page" we never want to show the App Banner
// when the side bar in in focus.
return state.ui.appBannerVisibility && getCurrentLayoutFocus( state ) !== 'sidebar';
}
1 change: 1 addition & 0 deletions client/state/ui/selectors/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,6 @@ export { default as getSectionName } from './get-section-name';
export { default as getSectionGroup } from './get-section-group';
export { default as isSiteSection } from './is-site-section';
export { default as isSectionLoading } from './is-section-loading';
export { default as appBannerIsEnabled } from './app-banner-is-visible';
export { default as masterbarIsVisible } from './masterbar-is-visible';
export { default as getSidebarIsCollapsed } from './sidebar-visibility';

0 comments on commit 8b7daf5

Please sign in to comment.