From cf6dbb26f3e3f7a9179c9ed5dec9287d189e0c9c Mon Sep 17 00:00:00 2001 From: Brett Sun Date: Wed, 6 Jan 2016 15:13:09 +0100 Subject: [PATCH 1/3] Fix loading of Facebook share button --- js/actions/facebook_actions.js | 14 ++++++ .../facebook_share_button.js | 50 +++++++++++++------ js/third_party/facebook.js | 17 +++++-- sass/ascribe_social_share.scss | 4 ++ 4 files changed, 65 insertions(+), 20 deletions(-) create mode 100644 js/actions/facebook_actions.js diff --git a/js/actions/facebook_actions.js b/js/actions/facebook_actions.js new file mode 100644 index 00000000..2e784fba --- /dev/null +++ b/js/actions/facebook_actions.js @@ -0,0 +1,14 @@ +'use strict'; + +import { altThirdParty } from '../alt'; + + +class FacebookActions { + constructor() { + this.generateActions( + 'sdkReady' + ); + } +} + +export default altThirdParty.createActions(FacebookActions); diff --git a/js/components/ascribe_social_share/facebook_share_button.js b/js/components/ascribe_social_share/facebook_share_button.js index aa0b6691..682bc6f1 100644 --- a/js/components/ascribe_social_share/facebook_share_button.js +++ b/js/components/ascribe_social_share/facebook_share_button.js @@ -2,6 +2,8 @@ import React from 'react'; +import FacebookHandler from '../../third_party/facebook'; + import AppConstants from '../../constants/application_constants'; import { InjectInHeadUtils } from '../../utils/inject_utils'; @@ -17,24 +19,40 @@ let FacebookShareButton = React.createClass({ }; }, - componentDidMount() { - /** - * Ideally we would only use FB.XFBML.parse() on the component that we're - * mounting, but doing this when we first load the FB sdk causes unpredictable behaviour. - * The button sometimes doesn't get initialized, likely because FB hasn't properly - * been initialized yet. - * - * To circumvent this, we always have the sdk parse the entire DOM on the initial load - * (see FacebookHandler) and then use FB.XFBML.parse() on the mounting component later. - */ - - InjectInHeadUtils - .inject(AppConstants.facebook.sdkUrl) - .then(() => { FB.XFBML.parse(this.refs.fbShareButton.getDOMNode().parentElement) }); + getInitialState() { + return FacebookHandler.getState(); }, - shouldComponentUpdate(nextProps) { - return this.props.type !== nextProps.type; + componentDidMount() { + FacebookHandler.listen(this.onChange); + + this.loadFacebook(); + }, + + shouldComponentUpdate(nextProps, nextState) { + // Don't update if the props haven't changed or the FB SDK loading status is still the same + return this.props.type !== nextProps.type || nextState.loaded !== this.state.loaded; + }, + + componentDidUpdate() { + // If the component changes, we need to reparse the share button's XFBML. + // To prevent cases where the Facebook SDK hasn't been loaded yet at this stage, + // let's make sure that it's injected before trying to reparse. + this.loadFacebook(); + }, + + onChange(state) { + this.setState(state); + }, + + loadFacebook() { + InjectInHeadUtils + .inject(AppConstants.facebook.sdkUrl) + .then(() => { + if (this.state.loaded) { + FB.XFBML.parse(this.refs.fbShareButton.getDOMNode().parent) + } + }); }, render() { diff --git a/js/third_party/facebook.js b/js/third_party/facebook.js index eab0b0e0..9e40e7ab 100644 --- a/js/third_party/facebook.js +++ b/js/third_party/facebook.js @@ -1,13 +1,18 @@ 'use strict'; import { altThirdParty } from '../alt'; + import EventActions from '../actions/event_actions'; +import FacebookActions from '../actions/facebook_actions'; import AppConstants from '../constants/application_constants' class FacebookHandler { constructor() { + this.loaded = false; + this.bindActions(EventActions); + this.bindActions(FacebookActions); } onApplicationWillBoot(settings) { @@ -16,15 +21,19 @@ class FacebookHandler { window.fbAsyncInit = () => { FB.init({ appId: AppConstants.facebook.appId, - // Force FB to parse everything on first load to make sure all the XFBML components are initialized. - // If we don't do this, we can run into issues with components on the first load who are not be - // initialized. - xfbml: true, + // Don't parse anything on the first load as we will parse all XFBML components as necessary. + xfbml: false, version: 'v2.5', cookie: false }); + + FacebookActions.sdkReady(); }; } + + onSdkReady() { + this.loaded = true; + } } export default altThirdParty.createStore(FacebookHandler, 'FacebookHandler'); diff --git a/sass/ascribe_social_share.scss b/sass/ascribe_social_share.scss index 0a577fb6..64e6e06b 100644 --- a/sass/ascribe_social_share.scss +++ b/sass/ascribe_social_share.scss @@ -7,4 +7,8 @@ width: 56px; box-sizing: content-box; /* We want to ignore padding in these calculations as we use the sdk buttons' height and width */ padding: 1px 0; + + &:not(:first-child) { + margin-left: 1px; + } } From 7f1e4ce0db150651b5c157b9b678abf09f473c1a Mon Sep 17 00:00:00 2001 From: Brett Sun Date: Wed, 6 Jan 2016 15:23:48 +0100 Subject: [PATCH 2/3] Rename third party handler files to match their declarations --- js/app.js | 17 ++++++----------- js/third_party/{debug.js => debug_handler.js} | 1 - .../{facebook.js => facebook_handler.js} | 0 js/third_party/{ga.js => ga_handler.js} | 0 .../{intercom.js => intercom_handler.js} | 0 ...otifications.js => notifications_handler.js} | 0 js/third_party/{raven.js => raven_handler.js} | 0 7 files changed, 6 insertions(+), 12 deletions(-) rename js/third_party/{debug.js => debug_handler.js} (99%) rename js/third_party/{facebook.js => facebook_handler.js} (100%) rename js/third_party/{ga.js => ga_handler.js} (100%) rename js/third_party/{intercom.js => intercom_handler.js} (100%) rename js/third_party/{notifications.js => notifications_handler.js} (100%) rename js/third_party/{raven.js => raven_handler.js} (100%) diff --git a/js/app.js b/js/app.js index dc8204cf..c4645296 100644 --- a/js/app.js +++ b/js/app.js @@ -6,9 +6,7 @@ import React from 'react'; import { Router, Redirect } from 'react-router'; import history from './history'; -/* eslint-disable */ import fetch from 'isomorphic-fetch'; -/* eslint-enable */ import ApiUrls from './constants/api_urls'; @@ -23,15 +21,13 @@ import { getSubdomain } from './utils/general_utils'; import EventActions from './actions/event_actions'; -/* eslint-disable */ // You can comment out the modules you don't need -// import DebugHandler from './third_party/debug'; -import GoogleAnalyticsHandler from './third_party/ga'; -import RavenHandler from './third_party/raven'; -import IntercomHandler from './third_party/intercom'; -import NotificationsHandler from './third_party/notifications'; -import FacebookHandler from './third_party/facebook'; -/* eslint-enable */ +// import DebugHandler from './third_party/debug_handler'; +import FacebookHandler from './third_party/facebook_handler'; +import GoogleAnalyticsHandler from './third_party/ga_handler'; +import IntercomHandler from './third_party/intercom_handler'; +import NotificationsHandler from './third_party/notifications_handler'; +import RavenHandler from './third_party/raven_handler'; initLogging(); @@ -105,4 +101,3 @@ class AppGateway { let ag = new AppGateway(); ag.start(); - diff --git a/js/third_party/debug.js b/js/third_party/debug_handler.js similarity index 99% rename from js/third_party/debug.js rename to js/third_party/debug_handler.js index 23fe4d04..cd11bf72 100644 --- a/js/third_party/debug.js +++ b/js/third_party/debug_handler.js @@ -4,7 +4,6 @@ import { altThirdParty } from '../alt'; import EventActions from '../actions/event_actions'; - class DebugHandler { constructor() { let symbols = []; diff --git a/js/third_party/facebook.js b/js/third_party/facebook_handler.js similarity index 100% rename from js/third_party/facebook.js rename to js/third_party/facebook_handler.js diff --git a/js/third_party/ga.js b/js/third_party/ga_handler.js similarity index 100% rename from js/third_party/ga.js rename to js/third_party/ga_handler.js diff --git a/js/third_party/intercom.js b/js/third_party/intercom_handler.js similarity index 100% rename from js/third_party/intercom.js rename to js/third_party/intercom_handler.js diff --git a/js/third_party/notifications.js b/js/third_party/notifications_handler.js similarity index 100% rename from js/third_party/notifications.js rename to js/third_party/notifications_handler.js diff --git a/js/third_party/raven.js b/js/third_party/raven_handler.js similarity index 100% rename from js/third_party/raven.js rename to js/third_party/raven_handler.js From 3984469f38883ceb4929fc9095bb5e9da61c53b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20Daubensch=C3=BCtz?= Date: Tue, 19 Jan 2016 09:10:33 +0100 Subject: [PATCH 3/3] Fix input for FacebookShareButton --- js/components/ascribe_social_share/facebook_share_button.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/js/components/ascribe_social_share/facebook_share_button.js b/js/components/ascribe_social_share/facebook_share_button.js index 682bc6f1..d5fbf699 100644 --- a/js/components/ascribe_social_share/facebook_share_button.js +++ b/js/components/ascribe_social_share/facebook_share_button.js @@ -2,7 +2,7 @@ import React from 'react'; -import FacebookHandler from '../../third_party/facebook'; +import FacebookHandler from '../../third_party/facebook_handler'; import AppConstants from '../../constants/application_constants';