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.

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

Eliminate no-global-event-listener ESLint warnings #5732

Merged
merged 4 commits into from
Dec 23, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,11 @@ const addBeforeUnloadPrompt = () => {
if ( beforeUnloadPromptAdded ) {
return;
}
window.addEventListener( 'beforeunload', onBeforeUnload );
global.addEventListener( 'beforeunload', onBeforeUnload );

// Remove prompt when clicking trash or update.
document.querySelector( '#major-publishing-actions' ).addEventListener( 'click', () => {
window.removeEventListener( 'beforeunload', onBeforeUnload );
global.removeEventListener( 'beforeunload', onBeforeUnload );
} );

beforeUnloadPromptAdded = true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ class ErrorRows {
} );
};

window.addEventListener( 'click', ( event ) => {
global.addEventListener( 'click', ( event ) => {
if ( toggleButtons.includes( event.target ) ) {
onButtonClick( event.target );
}
Expand Down
10 changes: 6 additions & 4 deletions assets/src/components/unsaved-changes-warning/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,10 @@ import { Options } from '../options-context-provider';
*
* @param {Object} props Component props.
* @param {boolean} props.excludeUserContext Whether to exclude listening to user context.
* @param {Element} props.appRoot React app root.
* @return {null} Renders nothing.
*/
export function UnsavedChangesWarning( { excludeUserContext = false } ) {
export function UnsavedChangesWarning( { excludeUserContext = false, appRoot } ) {
const { hasOptionsChanges, didSaveOptions } = useContext( Options );
const [ userState, setUserState ] = useState( { hasDeveloperToolsOptionChange: false, didSaveDeveloperToolsOption: true } );

Expand All @@ -36,19 +37,20 @@ export function UnsavedChangesWarning( { excludeUserContext = false } ) {
return null;
};

window.addEventListener( 'beforeunload', warnIfUnsavedChanges );
appRoot.ownerDocument.addEventListener( 'beforeunload', warnIfUnsavedChanges );

return () => {
window.removeEventListener( 'beforeunload', warnIfUnsavedChanges );
appRoot.ownerDocument.removeEventListener( 'beforeunload', warnIfUnsavedChanges );
};
}

return () => undefined;
}, [ hasOptionsChanges, didSaveOptions, hasDeveloperToolsOptionChange, didSaveDeveloperToolsOption ] );
}, [ appRoot, hasOptionsChanges, didSaveOptions, hasDeveloperToolsOptionChange, didSaveDeveloperToolsOption ] );

return excludeUserContext ? null : <WithUserContext setUserState={ setUserState } />;
}
UnsavedChangesWarning.propTypes = {
appRoot: PropTypes.instanceOf( global.Element ),
excludeUserContext: PropTypes.bool,
};

Expand Down
2 changes: 1 addition & 1 deletion assets/src/mobile-redirection.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
return;
}

document.addEventListener( 'DOMContentLoaded', () => {
global.addEventListener( 'DOMContentLoaded', () => {
// Show the mobile version switcher link once the DOM has loaded.
const siteVersionSwitcher = document.getElementById( 'amp-mobile-version-switcher' );
if ( ! siteVersionSwitcher ) {
Expand Down
2 changes: 1 addition & 1 deletion assets/src/onboarding-wizard/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ domReady( () => {
render(

<Providers>
<SetupWizard closeLink={ CLOSE_LINK } finishLink={ FINISH_LINK } />
<SetupWizard closeLink={ CLOSE_LINK } finishLink={ FINISH_LINK } appRoot={ root } />
</Providers>,
root,
);
Expand Down
6 changes: 4 additions & 2 deletions assets/src/onboarding-wizard/setup-wizard.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,9 @@ function PageComponentSideEffects( { children } ) {
* @param {Object} props Component props.
* @param {string} props.closeLink Link to return to previous user location.
* @param {string} props.finishLink Exit link.
* @param {Element} props.appRoot App root element.
*/
export function SetupWizard( { closeLink, finishLink } ) {
export function SetupWizard( { closeLink, finishLink, appRoot } ) {
const { isMobile } = useWindowWidth();
const { activePageIndex, currentPage: { title, PageComponent, showTitle }, moveBack, moveForward, pages } = useContext( Navigation );

Expand Down Expand Up @@ -98,12 +99,13 @@ export function SetupWizard( { closeLink, finishLink } ) {
/>
</div>
</div>
<UnsavedChangesWarning />
<UnsavedChangesWarning appRoot={ appRoot } />
</div>
);
}

SetupWizard.propTypes = {
closeLink: PropTypes.string.isRequired,
finishLink: PropTypes.string.isRequired,
appRoot: PropTypes.instanceOf( global.Element ),
};
12 changes: 9 additions & 3 deletions assets/src/settings-page/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,11 @@ function scrollFocusedSectionIntoView( focusedSectionId ) {

/**
* Settings page application root.
*
* @param {Object} props
* @param {Element} props.appRoot App root.
*/
function Root() {
function Root( { appRoot } ) {
const [ focusedSection, setFocusedSection ] = useState( global.location.hash.replace( /^#/, '' ) );

const { fetchingOptions, saveOptions } = useContext( Options );
Expand Down Expand Up @@ -187,10 +190,13 @@ function Root() {
</AMPDrawer>
<SettingsFooter />
</form>
<UnsavedChangesWarning excludeUserContext={ true } />
<UnsavedChangesWarning excludeUserContext={ true } appRoot={ appRoot } />
</>
);
}
Root.propTypes = {
appRoot: PropTypes.instanceOf( global.Element ),
};

domReady( () => {
const root = document.getElementById( 'amp-settings-root' );
Expand All @@ -199,7 +205,7 @@ domReady( () => {
render( (
<ErrorContextProvider>
<Providers>
<Root />
<Root appRoot={ root } />
</Providers>
</ErrorContextProvider>
), root );
Expand Down
4 changes: 2 additions & 2 deletions assets/src/utils/use-window-width.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,10 @@ export function useWindowWidth( args = {} ) {
setWidth( window.innerWidth );
};

window.addEventListener( 'resize', resizeCallback, { passive: true } );
global.addEventListener( 'resize', resizeCallback, { passive: true } );

return () => {
window.removeEventListener( 'resize', resizeCallback );
global.removeEventListener( 'resize', resizeCallback );
};
}, [] );

Expand Down