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

bug: Drawer causing overflow/scrollbar on right side of page on desktop #2859

Closed
AndrewB-S opened this issue Feb 19, 2024 · 13 comments
Closed

Comments

@AndrewB-S
Copy link

What version of daisyUI are you using?

4.7.2

Which browsers are you seeing the problem on?

Chrome, Firefox

Reproduction URL

https://play.tailwindcss.com/IQNlGqiMLf

Describe your issue

There is a bar that is created on the right side of the screen when the drawer is toggled open. In my own testing it happened with every version of the drawer available, with no other content present.

image

Copy link

Thank you @AndrewB-S for reporting issues. It helps daisyUI a lot 💚
I'll be working on issues one by one. I will help with this one as soon as a I find a solution.
In the meantime providing more details and reproduction links would be helpful.

@cqh963852
Copy link

cqh963852 commented Mar 4, 2024

@saadeghi

Encountered the same problem, reproduced the demo

  1. set the input as global variable
  2. set temp1.checked = true

image


It seems that there is something wrong with the following style

html:has(.drawer-toggle:checked) {

@cqh963852
Copy link

It seems that the root cause of the problem has been located.
Do daisyui need to add some automated tests?
Playwright can work in containers, maybe use it to do some snapshot comparison testing

@cqh963852
Copy link

cqh963852 commented Mar 4, 2024

The pipeline I have used.

It will create a commit for update snapshot, If test faild. Maybe submitting a PR will serve the purpose of automated testing.

Hope this helps.

name: CI Check

on:
  pull_request:
    branches:
      - main
  merge_group:
    branches:
      - main

jobs:
  ci-test:
    name: CI Check
    runs-on: ubuntu-latest
    steps:
      - name: Generate a token
        id: generate_token
        uses: tibdex/github-app-token@b62528385c34dbc9f38e5f4225ac829252d1ea92
        with:
          app_id: ${{ secrets.IDEALJS_BOT_APP_ID }}
          private_key: ${{ secrets.IDEALJS_BOT_PRIVATE_KEY }}
      - uses: actions/checkout@v3
        with:
          token: ${{ steps.generate_token.outputs.token }}
      - uses: actions/setup-node@v3
        with:
          node-version: 18
          cache: yarn
      - name: Install Dependencies
        run: yarn install --immutable
      - name: Test Packages
        run: yarn workspaces foreach run test
      - name: Build Packages
        run: yarn workspaces foreach run build
      - name: Install Playwright Browsers
        run: yarn workspace website playwright install --with-deps
      - name: Run Playwright tests
        id: e2e
        run: yarn workspace website test:e2e
      - name: Update Playwright Snapshot
        if: ${{ failure() && steps.e2e.conclusion == 'failure' }}
        run: yarn workspace website test:e2e -u
      - name: Upload playwright-report
        uses: actions/upload-artifact@v3
        if: always()
        with:
          path: apps/website/playwright-report/
          retention-days: 30
      - uses: stefanzweifel/git-auto-commit-action@v4
        if: ${{ failure() && steps.e2e.conclusion == 'failure' }}
        with:
          commit_message: 'chore: update image snapshot'

@saadeghi
Copy link
Owner

saadeghi commented Mar 4, 2024

That's scrollbar gutter.

Whether you like it or not there is a reason it exists: if the page has scrollable content, when we open the drawer it is expected for the page to stop being scrolled and it is expected for the sidebar to get scrolled. In this situation, if the page itself has a standard scrollbar (Windows, Linux, Mac if enable manually) it will suddenly hide the the scrollbar and causes the page to shift for the width of scrollbar. Scrollbar gutter is a CSS rule to avoid this layout shift. If there was a better solution I would use it of course but preserving the scrollbar width is totally more acceptable than a layout shift.

Let me know if you have any questions.

@saadeghi saadeghi closed this as completed Mar 4, 2024
@Vladastos
Copy link

Vladastos commented Mar 4, 2024

It looks like the issue might be the fact that the lines
html:has(.drawer-toggle:checked) { @apply overflow-y-hidden; scrollbar-gutter: stable; }
cause the scrollbar gutter to appear even when there was no scrollbar on the page, causing a layout shift.
At least that's what it was in my case.

A possible solution would be to find a way to make the make the gutter appear only when the html element is overflowing.
Hope it helps

@cqh963852
Copy link

when we open the drawer it is expected for the page to stop being scrolled and it is expected for the sidebar to get scrolled

I can understand the need for this feature.

But is there any way we can prevent scroll from displaying? May be drawer-side-no-scroll

If my content area is only 100vh. And the drawer-side will never overflow.

<div className="drawer">
  <input id="my-drawer" type="checkbox" className="drawer-toggle" />
  <div className="drawer-content">
    {/* Page content here */}
    <label htmlFor="my-drawer" className="btn btn-primary drawer-button">Open drawer</label>
  </div> 
--- <div className="drawer-side">
+++ <div className="drawer-side drawer-side-no-scroll">
    <label htmlFor="my-drawer" aria-label="close sidebar" className="drawer-overlay"></label>
    <ul className="menu p-4 w-80 min-h-full bg-base-200 text-base-content">
      {/* Sidebar content here */}
      <li><a>Sidebar Item 1</a></li>
      <li><a>Sidebar Item 2</a></li>
      
    </ul>
  </div>
</div>

@cqh963852
Copy link

cqh963852 commented Mar 5, 2024

From another perspective, I think combined is better than reduction. May be a drawer-side-sticky.

The following modification might be better?

---html:has(.drawer-toggle:checked) {
+++html:has(.drawer-toggle:checked ~ drawer-side-sticky) {
  @apply overflow-y-hidden;
  scrollbar-gutter: stable;
}
<div className="drawer">
  <input id="my-drawer" type="checkbox" className="drawer-toggle" />
  <div className="drawer-content">
    {/* Page content here */}
    <label htmlFor="my-drawer" className="btn btn-primary drawer-button">Open drawer</label>
  </div> 
--- <div className="drawer-side">
+++ <div className="drawer-side drawer-side-sticky">
    <label htmlFor="my-drawer" aria-label="close sidebar" className="drawer-overlay"></label>
    <ul className="menu p-4 w-80 min-h-full bg-base-200 text-base-content">
      {/* Sidebar content here */}
      <li><a>Sidebar Item 1</a></li>
      <li><a>Sidebar Item 2</a></li>
      
    </ul>
  </div>
</div>

@saadeghi
Copy link
Owner

saadeghi commented Mar 5, 2024

A possible solution would be to find a way to make the make the gutter appear only when the html element is overflowing.

If there's a way, let me know.

But is there any way we can prevent scroll from displaying? May be drawer-side-no-scroll

You can remove the scrollbar gutter using [scrollbar-gutter:auto] class

@cqh963852
Copy link

@saadeghi Sorry to bother you, it seems that this classname does not take effect. It will be override by the drawer's style

image
image
image

@cqh963852
Copy link

Using the following method solved my problem, but I still feel that the solution is not elegant enough

image
image

@yanickrochon
Copy link

That's scrollbar gutter.

[...] preserving the scrollbar width is totally more acceptable than a layout shift.

But when there are no vertical scrollbars, the gutter appears and makes the layout shift anyway. To me, and until CSS will support detecting content overflow, using JavaScript is a better solution.

Some preliminary test got this simple solution working (using Next.js) :

/* global.css */

html {
  scrollbar-gutter: auto !important;
}

html:has(body.content-overflow-y) {
  scrollbar-gutter: stable !important;
}
// ui.ts

const observer = new MutationObserver(() => {
  const body = document.body;
  const hasSscrollbar =
    body.scrollHeight != Math.max(body.offsetHeight, body.clientHeight);

  if (hasSscrollbar) {
    body.classList.add("content-overflow-y");
  } else {
    body.classList.remove("content-overflow-y");
  }
});

observer.observe(document.body, { childList: true, subtree: true });

That's it. The gutter only appears when there is a vertical scrollbar on document.body.

@D4LI3N
Copy link

D4LI3N commented Sep 30, 2024

I just added this in my app.css, and that did it:

html {
  scrollbar-gutter: auto !important;
}

html:has(body.content-overflow-y) {
  scrollbar-gutter: stable !important;
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants