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

Fix: make sidebar closed on mobile when page loads even if it was opened before the reload #6876

merged 1 commit into from May 25, 2018


None yet
2 participants

jorgefilipecosta commented May 21, 2018

On the desktop, we should persist the sidebar that was opened before. On mobile that should not apply and by default sidebars should be closed.
We regressed on this behavior because before the onChangeListener was invoking logic to close sidebar on mobile at the start, but during other unrelated change, the invocation stopped happening.

This PR adds a flag to onChangeListener that allows usages to explicitly set they want the listner invoked at the start.

How has this been tested?

Open Gutenberg on mobile, verify all sidebars are closed.
Open a sidebar, reload the page and see the sidebar got closed during the reload.

End to end test added in #6877.

export const onChangeListener = ( selector, listener ) => {
export const onChangeListener = ( selector, listener, executeAtStart = false ) => {

This comment has been minimized.


noisysocks May 23, 2018


I'm not sure about this param. It doesn't line up with e.g. how addEventListener works which is what I imagine most readers would expect.

Could we leave it up to the caller?

function adjustSidebar() {
	() => select( 'core/viewport' ).isViewportMatch( '< medium' ),

This comment has been minimized.


jorgefilipecosta May 24, 2018


Thank you for the feedback @noisysocks it was applied.



@jorgefilipecosta jorgefilipecosta merged commit d6b7334 into master May 25, 2018

2 checks passed

codecov/project 46.13% (-0.01%) compared to 321be98
continuous-integration/travis-ci/pr The Travis CI build passed

@jorgefilipecosta jorgefilipecosta deleted the fix/sidebar-closed-by-default-on-mobile branch May 25, 2018

@jorgefilipecosta jorgefilipecosta added this to the 3.0 milestone May 30, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment