Page MenuHomeSoftware Heritage

Fix sidebar not opening first time
ClosedPublic

Authored by kalpitk on Jun 14 2019, 12:29 PM.

Details

Summary

On devices with <992px width (sm, xs), sidebar doesn't open when swh-push-menu is clicked first time.
This was due to webapp-utils.js:5 setting collapseSidebar to false by default on all devices.

Diff Detail

Repository
rDWAPPS Web applications
Branch
sidebarFix
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 6243
Build 8633: tox-on-jenkinsJenkins
Build 8632: arc lint + arc unit

Event Timeline

This revision now requires changes to proceed.Jun 14 2019, 2:15 PM

Where is the value "992" coming from?

Where is the value "992" coming from?

bootstrap breakpoint
Below 992px means sm, xs devices.

This needs a test (after D1577 lands)

Yeah sure

bootstrap breakpoint
Below 992px means sm, xs devices.

Oh, right!
Can we/should we import the constant from bootstrap instead of hardcoding it here?

bootstrap breakpoint
Below 992px means sm, xs devices.

Oh, right!
Can we/should we import the constant from bootstrap instead of hardcoding it here?

We can't import these constants from bootstrap directly.

We can declare breakpoint constants globally for all the files.
If we want to have these constants for all the end to end tests ( where they will be mostly used), I think we can add the to index.js
Something like BP_XS, BP_SM, BP_MD, BP_LG may be globally declared constants

I'd prefer them to be named BREAKPOINT_* instead of BP_*, but that sounds nice, thanks

  • Add bootstrap breakpoints constants
  • Minor correction: md screen >=BREAKPOINT_MD rather than >BREAKPOINT_MD
anlambert added a subscriber: anlambert.

Just a small change to do and I will accept the diff.

swh/web/assets/src/bundles/webapp/webapp-utils.js
27–30

You should move this after line 6. Indeed, we backup the state of the sidebar in the localStorage to restore it when visiting the Software Heritage archive website again. Otherwise, I am OK with this fix.

This revision now requires changes to proceed.Jun 17 2019, 10:47 AM
swh/web/assets/src/bundles/webapp/webapp-utils.js
27–30

But if the device is sm or xs we don't restore state from local storage. (Sidebar is always collapsed when you open any page irrespective of previous state)

anlambert added inline comments.
swh/web/assets/src/bundles/webapp/webapp-utils.js
27–30

Indeed, you are right. So this can be landed as is.

This revision now requires review to proceed.Jun 17 2019, 11:51 AM
This revision is now accepted and ready to land.Jun 17 2019, 3:13 PM
This revision was automatically updated to reflect the committed changes.