Add helpscout's embedded script #758
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,67 @@ | ||
| import React, { useEffect } from 'react' | ||
| import PropTypes from 'prop-types' | ||
| import { Helmet } from 'react-helmet' | ||
| import { noop, GU } from '../../utils' | ||
|
|
||
| const BEACON_EMBED = | ||
| '!function(e,t,n){function a(){var e=t.getElementsByTagName("script")[0],n=t.createElement("script");n.type="text/javascript",n.async=!0,n.src="https://beacon-v2.helpscout.net",e.parentNode.insertBefore(n,e)}if(e.Beacon=n=function(t,n,a){e.Beacon.readyQueue.push({method:t,options:n,data:a})},n.readyQueue=[],"complete"===t.readyState)return a();e.attachEvent?e.attachEvent("onload",a):e.addEventListener("load",a,!1)}(window,document,window.Beacon||function(){});' | ||
| const HELPSCOUT_ID = "'163e0284-762b-4e2d-b3b3-70a73a7e6c9f'" | ||
| const BEACON_INIT = "window.Beacon('init'," + HELPSCOUT_ID + ')' | ||
|
|
||
| const BeaconHeadScripts = React.memo(({ optedIn, onReady }) => { | ||
| useEffect(() => { | ||
| let timeout = null | ||
| if (optedIn) { | ||
| ;(function isBeaconReady() { | ||
| if (window.Beacon) { | ||
| onReady() | ||
| return | ||
| } | ||
| timeout = setTimeout(isBeaconReady, 100) | ||
| })() | ||
|
|
||
| return () => clearTimeout(timeout) | ||
| } | ||
| }, [optedIn, onReady]) | ||
|
|
||
| if (!optedIn) { | ||
| return null | ||
| } | ||
|
|
||
| return ( | ||
| <Helmet> | ||
|
There was a problem hiding this comment. What would you think about trying to skip // We could use styled-components’ createGlobalStyle for CSS
const HelpscoutStyle = createGlobalStyle`
/* … */
`
// And this could replace the two inline scripts
function initBeacon(helpscoutId) {
if (window.Beacon) {
return window.Beacon
}
window.Beacon = (method, options, data) => {
if (window.Beacon.readyQueue) {
window.Beacon.readyQueue.push({ method, options, data })
}
}
window.Beacon.readyQueue = [{ method: 'init', options: helpscoutId }]
const insertScript = () => {
const script = document.createElement('script')
script.async = true
script.src = 'https://beacon-v2.helpscout.net'
document.body.appendChild(script)
}
if (document.readyState === 'complete') {
insertScript()
} else {
window.addEventListener('load', insertScript)
}
return window.Beacon
}
function useHelpScoutBeacon(optedIn) {
const [beacon, setBeacon] = useState(null)
useEffect(() => {
if (optedIn && !beacon) {
setBeacon(() => initBeacon(HELPSCOUT_ID))
}
// Here we could destroy it, but I’m not sure if HelpScout’s
// destroy / init actions are fully sync, so better leave it always on once it’s here.
// return () => if (beacon) beacon('destroy')
}, [optedIn, beacon])
return beacon
}
// it might not make sense at this point to keep this component :D
const BeaconHeadScripts = ({ optedIn }) => {
useHelpScoutBeacon(optedIn)
return optedIn ? <HelpscoutBeaconStyle /> : null
}There was a problem hiding this comment. Your suggestion is a specific implementation for There was a problem hiding this comment.
It’s doing what their inline script is doing, so I’m not too worried about that. The important thing it gives us is control over it, so we can know exactly when it is ready. But it’s true that it might be a problem in the future if they ask their users to update the init script, so I let you decide!
I understand, but I really hope injecting anything else is not going to be needed! Until then I consider this to be an exception to the rule, but running code that we don’t control relying on a remote HTTP server is something we should avoid as much as possible. One thing we could still do to avoid using // /!\ Untested code
function useHelpScoutBeacon(optedIn) {
const [beacon, setBeacon] = useState(null)
// Load the script if it doesn’t exist yet and optedIn
// is true, then set the value of Beacon.
useEffect(() => {
let scripts = []
if (optedIn && !beacon) {
if (!window.Beacon) {
scripts = [BEACON_EMBED, BEACON_INIT].map(source => {
const script = document.createElement('script')
script.innerHTML = source
return document.body.appendChild(script)
})
}
setBeacon(window.Beacon)
}
return () => scripts.forEach(s => s.remove())
}, [optedIn, beacon])
return beacon
}There was a problem hiding this comment.
Agree 100%, but for a feature that I understand to be "let them have it and see if they use it", let's not worry about being too optimized for now :). There was a problem hiding this comment. Yes totally, I should have mentioned here that the original intention was to avoid using To rephrase my comment: I don’t think we need a generic mechanism to load external scripts (either There was a problem hiding this comment. If we end up removing this feature then removing There was a problem hiding this comment. Let’s keep it for now, but it’s definitely a a module we should try to eliminate IMO: it’s easy to replace with a simple |
||
| <script type="text/javascript">{BEACON_EMBED}</script> | ||
| <script type="text/javascript">{BEACON_INIT}</script> | ||
| <style> | ||
| {` | ||
| .BeaconFabButtonFrame, | ||
| #beacon-container .Beacon div:first-of-type { | ||
| display: none !important; | ||
| } | ||
| @media (min-width: 768px) { | ||
| #beacon-container .BeaconContainer { | ||
| height: 600px !important; | ||
| width: 350px !important; | ||
| top: unset !important; | ||
| left: unset !important; | ||
| bottom: 80px !important; | ||
| right: ${3 * GU}px !important; | ||
| } | ||
| } | ||
| `} | ||
| </style> | ||
| </Helmet> | ||
| ) | ||
| }) | ||
|
|
||
| BeaconHeadScripts.propTypes = { | ||
| optedIn: PropTypes.bool, | ||
| onReady: PropTypes.func, | ||
| } | ||
|
|
||
| BeaconHeadScripts.defaultProps = { | ||
| optedIn: false, | ||
| onReady: noop, | ||
| } | ||
|
|
||
| export default BeaconHeadScripts | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably make this an environment variable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is not a secret, in the end it is part of your frontend if that is your concern.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It’s more that it’s a dynamic part, that (IMO) belong outside of the source code. For example, we should be able to change it without releasing a new version, or we might want to let people changing it for their own (e.g. in case of a fork).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we have a mechanism for something like this? Or set up manually adding the
envvar to the build process and dev?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do! https://github.com/aragon/aragon/blob/master/src/local-settings.js
Maybe we could also not render it if the variable is not set?