Fix mobile sidebar scrolling issue

[arnellebalane]

What type of PR is this? (check all applicable)

• Refactor
• Feature
• Bug Fix
• Documentation Update

Description

When the sidebar is opened in mobile layout, the darkened background (.sidebar-bg) is longer than the actual sidebar element itself, making it possible to scroll past the bottom of the actual sidebar.

The cause of the issue is the height: 1000%; declaration set in the .sidebar-bg element. This PR refactors the sidebar behavior to not rely on the .sidebar-bg element as the darkened background. The darkened background is moved to the .side-bar element as box-shadow, and the .sidebar-bg elements are removed, ensuring that scrolling will not go past the bottom of the sidebar. ensures that the sidebar wrapper and .sidebar-bg elements are sized correctly, and that the sidebar does not overflow the wrapper. With the sidebar always within the wrapper, the darkened background will not scroll as well.

Related Tickets & Documents

#1746

Mobile & Desktop Screenshots/Recordings (if there are UI changes)

UI stays the same after the changes are applied.

Added to documentation?

• docs.dev.to
• readme
• no documentation needed

[CLAassistant]

All committers have signed the CLA.

[Link2Twenty]

I don't think it needs to be quite so drastic, I think these changes should fix the issue.

.home .sidebar-wrapper {
  height: calc(100vh - 48px);
}

.home .sidebar-wrapper .sidebar-bg {
  /* remove height: 1000%; */
}

.home .sidebar-wrapper.swiped-in .side-bar { 
  height: calc(100% - 2px);
  overflow-y: auto;
  /* remove min-height: calc(100vh - 44px); */
  /* remove top: 0; */
  /* remove bottom: 0; */
}

All can be found here

https://github.com/thepracticaldev/dev.to/blob/9e4d9ecac16a70cbe64c351506cecf3de5e12459/app/assets/stylesheets/articles.scss#L688-L726

[arnellebalane]

@Link2Twenty Apologies for the drastic changes to fix the issue 🙏 I've reverted my changes and applied your suggestion, which is much more simpler than what I originally did 😄

[Link2Twenty]

@arnellebalane No need to apologise. Your solution worked too after all 🙂.

The CSS solution does have a side effect in which the position of the sidebar is remembered when it is closed and opened again. So we might want to add something to JS to scroll back up on close, though I'm not sure that's crucial.

[Link2Twenty]

This is the code the controls the opening and closing of the sidebars

https://github.com/thepracticaldev/dev.to/blob/65110550d8c2231f73ff99ae305a46b7b0bcb4f9/app/assets/javascripts/utilities/slideSidebar.js#L1-L18

Adding this line should make the side-bar scroll back up on close, provided that's the expected behavour.

function slideSidebar(side,direction){ 
   if (!document.getElementById("sidebar-wrapper-"+side)){ 
     return; 
   } 
   if (direction === "intoView") { 
     document.getElementById("articles-list").classList.add("modal-open"); 
     document.getElementsByTagName("body")[0].classList.add("modal-open"); 
     document.getElementById("sidebar-wrapper-"+side).classList.add("swiped-in") 
     document.getElementById("sidebar-wrapper-"+side).scrollTop=0 
     document.getElementById("articles-list").addEventListener("touchmove", preventDefaultAction, false); 
   } 
   else { 
     document.getElementById("articles-list").classList.remove("modal-open"); 
     document.getElementsByTagName("body")[0].classList.remove("modal-open"); 
     document.getElementById("sidebar-wrapper-"+side).classList.remove("swiped-in"); 
     // New line!
     document.getElementById("sidebar-wrapper-"+side).querySelector('.side-bar').scroll(0,0); 
     document.getElementById("articles-list").removeEventListener("touchmove", preventDefaultAction, false); 
   } 
 }

[arnellebalane]

@Link2Twenty I will also try to add scrolling back the sidebar to the top on close in this PR, that sounds like the behavior that I would expect from the sidebar as well.

[Zhao-Andy]

Hey @arnellebalane, sounds like you're still working on this so I'm going to add WIP to the title. Feel free to change it and leave a comment when it's ready for review again.

[arnellebalane]

@Zhao-Andy Yes I'm still working on this. Thank you! 👍

[arnellebalane]

@Link2Twenty I have updated this PR to make the sidebar scroll back to top when they are closed. The diff for the JS file that I changed will show a lot of changes because ESLint automatically fixed most of the linting issues on that file upon committing my changes 😄

[Zhao-Andy]

Looks great! Tested on Android Firefox and Chrome, and iOS Safari.