material-components / material-web

Material Design Web Components
https://material-web.dev
Apache License 2.0
9.09k stars 860 forks source link

md-dialog: scrollbar Flash on opening #5121

Open vdegenne opened 10 months ago

vdegenne commented 10 months ago

What is affected?

Component

Description

The scrollbar flashes when the dialog opens even in the case the content is not scrollable. It's particularly more apparent in dark mode. See below playground for a demonstration of this issue.

Reproduction

https://lit.dev/playground/#gist=825f3acbc05233df00a8cdb1bd9693c2

Workaround

Could this issue be fixed by hidding the overflow on animation start?

Is this a regression?

No or unsure. This never worked, or I haven't tried before.

Affected versions

@material/web@1.0.1

Browser/OS/Node environment

Any browser/system

asyncLiz commented 10 months ago

I'm not able to reproduce on Chrome macOS. Which browser/os environment are you seeing the issue? Can you upload a screen recording too?

christophe-g commented 10 months ago

I can reproduce on Linux Mint 21.2 Chrome 118.0.5993.88.

I cannot reproduce on FF.

https://github.com/material-components/material-web/assets/1168053/871fb0f6-b29b-497c-a96c-fce37b3e8f1c

SummersRA commented 10 months ago

I can reproduce on Windows with Edge. No issue with FF.

vdegenne commented 10 months ago

Firefox is doing great (you can even select text in the dialogs 😹)

Basyras commented 6 months ago

Still can reproduce in Edge - Version 121.0.2277.98 (Official build) (64-bit)

SDBonhof commented 6 months ago

Can reproduce this with Chrome M114 and M122 running through Electron V0.25.X and V0.29.X, respectively

shhdharmen commented 4 months ago

I found an interim fix, adding overflow: visible to the .scroller seems to be fix the issue. Not sure what side-effects will it cause, but works fine for small dialogs.

function fixScrollFlash(mdDialog: MdDialog) {
    const shadowRoot = mdDialog.shadowRoot;
    if (shadowRoot) {
      const scroller = shadowRoot.querySelector<HTMLElement>(".scroller");
      if (scroller) {
        scroller.style.overflow = "visible";
      } else {
        console.log("scrollernot found");
      }
    } else {
      console.log("shadowRoote not found");
    }
  }
Basyras commented 3 months ago

My workaround. Slightly inspired by @shhdharmen workaround.

Using these 2 events it will make sure the scrollbar will work as needed after animation is played and the shadowRootchildren are present - so no check for .scroller shouldn't be needed.

open: Dispatched when the dialog is opening before any animations. opened: Dispatched when the dialog has opened after any animations.

static showDialog(dialog) {
    dialog.addEventListener("open", () => {
        this.changeScrollerOverflow(dialog, "visible");
    });

    dialog.addEventListener("opened", () => {
        this.changeScrollerOverflow(dialog, "hidden");
    });

    dialog.show();
}

static changeScrollerOverflow(mdDialog, overflowValue) {
    const shadowRoot = mdDialog.shadowRoot;
    const scroller = shadowRoot.querySelector(".scroller");
    scroller.style.overflow = overflowValue;
}