Clarity: Single action click of datagrid will cause scroll auto jump if whole page only has one scrollbar and it is on body

Created on 1 Dec 2020  路  12Comments  路  Source: vmware/clarity

Describe the bug

Single action click of datagrid will cause scroll auto jump if whole page only has one scrollbar and it is on body

How to reproduce

Steps to reproduce the behavior:

  1. Scroll down the page
  2. Click on single action icon
  3. Scrollbar auto jump to top

Expected behavior

Scrollbar should keep current position

Versions

App

  • Angular: 9.1.12
  • Node: 10.16.1
  • Clarity: 3.1.5

Device:

Chrome

@clangular datagrid needs info needs reproduction

Most helpful comment

@coryrylan can you review this week?

All 12 comments

Hi @sheguloves, can you please provide us a stackblitz example? Thanks!

same issue here

It sounds like the issue here is when a datagrid is not placed inside the .content-area as described in the application layout docs.

We recommend putting the datagrid inside that container to get proper style and behavior. If this isn't possible please create an isolated reproduction and give us as much details for the use case that forces the app to put it outside of the recommended container. We keep starter applications here for isolating and reproducing issues.

It sounds like the issue here is when a datagrid is not placed inside the .content-area as described in the application layout docs.

We recommend putting the datagrid inside that container to get proper style and behavior. If this isn't possible please create an isolated reproduction and give us as much details for the use case that forces the app to put it outside of the recommended container. We keep starter applications here for isolating and reproducing issues.

https://stackblitz.com/edit/clarity-v4-light-theme-fakdmw? the code is something like this

That stackblitz isn't running for me, even if I disable Ivy.
Here is a forked version that disables ivy and gets around the ngcc issue in stackblitz. Please update it to compile and run without errors so that I can take a look.

That stackblitz isn't running for me, even if I disable Ivy.
Here is a forked version that disables ivy and gets around the ngcc issue in stackblitz. Please update it to compile and run without errors so that I can take a look.

so sorry @hippee-lee, u can see the issue here: https://stackblitz.com/edit/gh-5342-datagrid-scrolling-issue-report-9x6f2g

That stackblitz isn't running for me, even if I disable Ivy.
Here is a forked version that disables ivy and gets around the ngcc issue in stackblitz. Please update it to compile and run without errors so that I can take a look.

so sorry @hippee-lee, u can see the issue here: https://stackblitz.com/edit/gh-5342-datagrid-scrolling-issue-report-9x6f2g

@xlucvvs Thanks for your stackblitz example. It works and shows the issue.

@hippee-lee I checked the code and found that the issue is caused by action item focus. When action pop up, Clarity focus the first item and it cause the issue.

The root cause is following

.datagrid-action-overflow {
    position: absolute; // To make action popover keep current related postion with the row when scrolling
}

Once datagrid action overflow DOM element is appended to document.body, the following code will be executed. Then the datagrid action overflow DOM element is focused with absolute position. If document.body scrollbar shows with scrolltop is none-zero, then scrollbar will jump to top.

focus() {
    if (isPlatformBrowser(this.platformId)) {
      this.renderer.setAttribute(this.el.nativeElement, 'tabindex', '0');
      this.el.nativeElement.focus();
    }
  }

@sheguloves doesn't work here, I found this line in 4 files, even commenting on all the files it didn't work.

@sheguloves doesn't work here, I found this line in 4 files, even commenting on all the files it didn't work.

@xlucvvs Sorry for miss leading. I just found the root cause.

focus() {
    if (isPlatformBrowser(this.platformId)) {
      this.renderer.setAttribute(this.el.nativeElement, 'tabindex', '0');
      this.el.nativeElement.focus();  // Set focus will cause scrollbar jumping
    }
  }
.datagrid-action-overflow {
      position: absolute;  // Related with focus
}

As comments says, change position to fixed to fix scrollbar jumping with the disadvantage: datagrid-action-overflow DOM element will keep current position which means it will not be aligned with datagrid row when datagrid is scrolling.

@sheguloves I think that is some problem with the element's scrollToClose, 'cause in the Datagrid documentation, you can see that if you scroll the page, the element will be hidden. Looking in the file you mentioned above, I found the line of code:
* clrPopoverContent =" open; at: smartPosition; outsideClickToClose: true; scrollToClose: true
Where scrollToClose is probably not working...

@coryrylan can you review this week?

@sheguloves I think that is some problem with the element's scrollToClose, 'cause in the Datagrid documentation, you can see that if you scroll the page, the element will be hidden. Looking in the file you mentioned above, I found the line of code:
* clrPopoverContent =" open; at: smartPosition; outsideClickToClose: true; scrollToClose: true
Where scrollToClose is probably not working...

Hi @xlucvvs

I downloaded the sample and ran it on my local environment with Clarity V3.

datagrid-action-overflow.ts

private focusFirstButton(): void {
    if (isPlatformBrowser(this.platformId)) {
      this.zone.runOutsideAngular(() => {
        setTimeout(() => {
          const firstButton: HTMLButtonElement = document.querySelector('button.action-item');
          if (firstButton) {
            // firstButton.focus = () => {} will not cause the bug. The root cause is still focus make scrollbar auto jumping.
            firstButton.focus();
          }
        });
      });
    }
  }
Was this page helpful?
0 / 5 - 0 ratings

Related issues

markofranjic picture markofranjic  路  22Comments

hippee-lee picture hippee-lee  路  25Comments

vmwareux-vivian picture vmwareux-vivian  路  41Comments

gracesnoh picture gracesnoh  路  41Comments

whizkidwwe1217 picture whizkidwwe1217  路  45Comments