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.

Sign up for GitHub

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(sidebar): track user pref for sidebar state #1394

Merged
merged 1 commit into from
Sep 12, 2021
Merged

fix(sidebar): track user pref for sidebar state #1394

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
66 changes: 53 additions & 13 deletions assets/js/sidebar/sidebar-drawer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,12 @@ const ANIMATION_DURATION = 300
const SIDEBAR_TOGGLE_SELECTOR = '.sidebar-toggle'
const CONTENT_SELECTOR = '.content'

const userPref = {
CLOSED: 'closed',
OPEN: 'open',
NO_PREF: 'no_pref'
}

const SIDEBAR_CLASS = {
opened: 'sidebar-opened',
opening: 'sidebar-opening',
Expand All @@ -20,7 +26,9 @@ const state = {
// Keep track of the current timeout to clear it if needed
togglingTimeout: null,
// Record window width on resize to update sidebar state only when it actually changes
lastWindowWidth: window.innerWidth
lastWindowWidth: window.innerWidth,
// No_PREF is defaults to OPEN behavior
sidebarPreference: userPref.NO_PREF
}

/**
Expand All @@ -32,11 +40,7 @@ export function initialize () {
}

function setDefaultSidebarState () {
setClass(
isScreenSmall()
? SIDEBAR_CLASS.closed
: SIDEBAR_CLASS.opened
)
setClass(isScreenSmall() ? SIDEBAR_CLASS.closed : SIDEBAR_CLASS.opened)
}

function isScreenSmall () {
Expand All @@ -49,17 +53,21 @@ function setClass (...classes) {
}

function addEventListeners () {
qs(SIDEBAR_TOGGLE_SELECTOR).addEventListener('click', event => {
qs(SIDEBAR_TOGGLE_SELECTOR).addEventListener('click', (event) => {
toggleSidebar()
setPreference()
})

qs(CONTENT_SELECTOR).addEventListener('click', event => {
qs(CONTENT_SELECTOR).addEventListener('click', (event) => {
closeSidebarIfSmallScreen()
})

window.addEventListener('resize', throttle(event => {
adoptSidebarToWindowSize()
}, 100))
window.addEventListener(
'resize',
throttle((event) => {
adoptSidebarToWindowSize()
}, 100)
)
}

/**
Expand All @@ -76,8 +84,10 @@ export function toggleSidebar () {
}

function isSidebarOpen () {
return document.body.classList.contains(SIDEBAR_CLASS.opened) ||
return (
document.body.classList.contains(SIDEBAR_CLASS.opened) ||
document.body.classList.contains(SIDEBAR_CLASS.opening)
)
}

/**
Expand Down Expand Up @@ -121,11 +131,23 @@ function clearTimeoutIfAny () {
}
}

/**
* Handles updating the sidebar state on window resize
*
* WHEN the window width has changed
* AND the user sidebar preference is OPEN or NO_PREF
* THEN adjust the sidebar state according to screen size
*/
function adoptSidebarToWindowSize () {
// See https://github.com/elixir-lang/ex_doc/issues/736#issuecomment-307371291
if (state.lastWindowWidth !== window.innerWidth) {
state.lastWindowWidth = window.innerWidth
setDefaultSidebarState()
if (
state.sidebarPreference === userPref.OPEN ||
state.sidebarPreference === userPref.NO_PREF
) {
setDefaultSidebarState()
}
}
}

Expand All @@ -135,3 +157,21 @@ function closeSidebarIfSmallScreen () {
closeSidebar()
}
}

/**
* Track the sidebar preference for the user
*/
function setPreference () {
switch (state.sidebarPreference) {
case userPref.OPEN:
state.sidebarPreference = userPref.CLOSED
break
case userPref.CLOSED:
state.sidebarPreference = userPref.OPEN
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this correct? If the user preference is closed, we are setting it to open? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The control logic is correct. The function is called when the click event tied to the toggle button fires. So whenever the user changes the state of the sidebar with the toggle button, we'll update their preference.

So, if you're preference was closed we'll set it open

And if you're preference was open, we'll set it to closed

And if you had no_pref, we'll set the pref to the state of the sidebar.

I've attached a short video showing the state changes while working with the sidebar

CB576ED3-41C0-4588-AC86-38EE7E9A2702.MP4

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would it be better if I called it togglePreference() instead of setPreference()

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Beautiful video, thank you! 😍

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would it be better if I called it togglePreference() instead of setPreference()

Ah, good idea! I just merged the PR though, but feel free to submit an update. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will do

break
case userPref.NO_PREF:
isSidebarOpen()
? (state.sidebarPreference = userPref.OPEN)
: (state.sidebarPreference = userPref.CLOSED)
}
}