Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Boot: Don't render empty Layout #26944

Merged
merged 5 commits into from
Nov 5, 2018
Merged

Boot: Don't render empty Layout #26944

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a115a4d
Boot: Don't render empty Layout
ockham Aug 28, 2018
4c99b20
Framework: Don't auto-inject page.js' onClick handler
ockham Oct 1, 2018
1bcdcf1
Layout: Manually add page.js' onClick handler
ockham Oct 2, 2018
f8fc160
Install the page.clickHandler on the root layout element
jsnajdr Oct 31, 2018
e5b64d2
Render empty layout for logged out home page
jsnajdr Nov 2, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion client/boot/app.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ const boot = currentUser => {
setupMiddlewares( currentUser, reduxStore );
invoke( project, 'setupMiddlewares', currentUser, reduxStore );
detectHistoryNavigation.start();
page.start( { decodeURLComponents: false } );
page.start( { click: false, decodeURLComponents: false } );
} );
};

Expand Down
8 changes: 5 additions & 3 deletions client/boot/common.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import { getSections } from 'sections-helper';
import { checkFormHandler } from 'lib/protect-form';
import notices from 'notices';
import authController from 'auth/controller';
import { makeLayout, render as clientRender } from 'controller';

const debug = debugFactory( 'calypso' );

Expand Down Expand Up @@ -102,9 +103,10 @@ const loggedOutMiddleware = currentUser => {
}
} );
} else if ( config.isEnabled( 'devdocs/redirect-loggedout-homepage' ) ) {
page( '/', () => {
page.redirect( '/devdocs/start' );
} );
page( '/', '/devdocs/start' );
} else {
// render an empty layout with masterbar links for logged-out home page
page( '/', makeLayout, clientRender );
}

const validSections = getSections().reduce( ( acc, section ) => {
Expand Down
71 changes: 25 additions & 46 deletions client/boot/project/wordpress-com.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@
* External dependencies
*/
import { startsWith } from 'lodash';
import React from 'react';
import ReactDom from 'react-dom';
import store from 'store';
import page from 'page';
import debugFactory from 'debug';
Expand All @@ -31,22 +29,9 @@ import { setNextLayoutFocus, activateNextLayoutFocus } from 'state/ui/layout-foc
import Logger from 'lib/catch-js-errors';
import setupMySitesRoute from 'my-sites';
import setupGlobalKeyboardShortcuts from 'lib/keyboard-shortcuts/global';
import * as controller from 'controller';

const debug = debugFactory( 'calypso' );

function renderLayout( reduxStore ) {
const Layout = controller.ReduxWrappedLayout;

const layoutElement = React.createElement( Layout, {
store: reduxStore,
} );

ReactDom.render( layoutElement, document.getElementById( 'wpcom' ) );

debug( 'Main layout rendered.' );
}

export const configureReduxStore = ( currentUser, reduxStore ) => {
debug( 'Executing WordPress.com configure Redux store.' );

Expand Down Expand Up @@ -74,38 +59,32 @@ export function setupMiddlewares( currentUser, reduxStore ) {
analytics.setSuperProps( superProps );
}

// Render Layout only for non-isomorphic sections.
// Isomorphic sections will take care of rendering their Layout last themselves.
if ( ! document.getElementById( 'primary' ) ) {
renderLayout( reduxStore );

if ( config.isEnabled( 'catch-js-errors' ) ) {
const errorLogger = new Logger();
//Save errorLogger to a singleton for use in arbitrary logging.
require( 'lib/catch-js-errors/log' ).registerLogger( errorLogger );
//Save data to JS error logger
errorLogger.saveDiagnosticData( {
user_id: currentUser.get().ID,
calypso_env: config( 'env_id' ),
} );
errorLogger.saveDiagnosticReducer( function() {
const state = reduxStore.getState();
return {
blog_id: getSelectedSiteId( state ),
calypso_section: getSectionName( state ),
};
} );
errorLogger.saveDiagnosticReducer( () => ( { tests: getSavedVariations() } ) );
analytics.on( 'record-event', ( eventName, eventProperties ) =>
errorLogger.saveExtraData( { lastTracksEvent: eventProperties } )
if ( config.isEnabled( 'catch-js-errors' ) && ! document.getElementById( 'primary' ) ) {
Copy link
Contributor Author

@ockham ockham Aug 28, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've retained the ! document.getElementById( 'primary' ) criterion here, since the catch-js-errors stuff was originally wrapped in it, but I have a hunch that it's not really required for it.

Of note, document.getElementById( 'primary' ) isn't always false -- isomorphic sections (such as /themes) render a component hierarchy on the server side, so the client will find a #primary div. (However, the rationale found in the PR desc still holds -- all individual sections render their own Layout component hierarchy, so we don't need to render an empty Layout here.)

const errorLogger = new Logger();
//Save errorLogger to a singleton for use in arbitrary logging.
require( 'lib/catch-js-errors/log' ).registerLogger( errorLogger );
//Save data to JS error logger
errorLogger.saveDiagnosticData( {
user_id: currentUser.get().ID,
calypso_env: config( 'env_id' ),
} );
errorLogger.saveDiagnosticReducer( function() {
const state = reduxStore.getState();
return {
blog_id: getSelectedSiteId( state ),
calypso_section: getSectionName( state ),
};
} );
errorLogger.saveDiagnosticReducer( () => ( { tests: getSavedVariations() } ) );
analytics.on( 'record-event', ( eventName, eventProperties ) =>
errorLogger.saveExtraData( { lastTracksEvent: eventProperties } )
);
page( '*', function( context, next ) {
errorLogger.saveNewPath(
context.canonicalPath.replace( getSiteFragment( context.canonicalPath ), ':siteId' )
);
page( '*', function( context, next ) {
errorLogger.saveNewPath(
context.canonicalPath.replace( getSiteFragment( context.canonicalPath ), ':siteId' )
);
next();
} );
}
next();
} );
}

// If `?sb` or `?sp` are present on the path set the focus of layout
Expand Down
9 changes: 8 additions & 1 deletion client/layout/index.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import React, { Component } from 'react';
import { connect } from 'react-redux';
import classnames from 'classnames';
import page from 'page';

/**
* Internal dependencies
Expand Down Expand Up @@ -59,6 +60,11 @@ class Layout extends Component {
colorSchemePreference: PropTypes.string,
};

// Intercepts <a href> clicks in the document and passes them to the `page` router to handle.
// If the link is internal to Calypso, the router will handle the navigation with `pushState`
// instead of letting the browser reload the whole app by performing a classic navigation.
pageClickHandler = e => page.clickHandler( e.nativeEvent );

render() {
const sectionClass = classnames(
'layout',
Expand All @@ -78,7 +84,8 @@ class Layout extends Component {
} );

return (
<div className={ sectionClass }>
// eslint-disable-next-line jsx-a11y/click-events-have-key-events, jsx-a11y/no-static-element-interactions
<div className={ sectionClass } onClick={ this.pageClickHandler }>
<DocumentHead />
<QuerySites primaryAndRecent />
<QuerySites allSites />
Expand Down