SAP / ui5-webcomponents-react

A wrapper implementation for React of the UI5 Web Components that are compliant with the SAP Fiori User Experience
https://sap.github.io/ui5-webcomponents-react/
Apache License 2.0
443 stars 99 forks source link

AnalyticalTable: Shows horizontal scrollbar when it shouldn't #3376

Closed ej612 closed 12 months ago

ej612 commented 2 years ago

Describe the bug We have a SplitterLayout with a vertical splitter and two SplitterElements. The right-hand SplitterElement contains an empty div, the left-hand SplitterElement contains an AnalyticalTable. The table has inlineSize: '100%' set on it so it fills the entire SplitterElement (horizontally). This lets us resize the table. If we move the splitter, we will see that as the AnalyticalTable resizes, it will sometimes display a horizontal scrollbar, sometimes it won't.

I have not been able to figure out a pattern. At first, I suspected it has something to do with the fact that if the table has a floating point width like e.g. 802.8px, each row object will have the width rounded up, in this case 803px. But this doesn't seem to be the problem, I for example see a scrollbar for 802.8px/803px, but not for 799.8px/800px.

I've attached screenshots showing the width of the table header (floating point width) vs the table header row (rounded width), in case it has something to do with that.

Isolated Example If it worked, here's a sandbox. If it didn't, here's the code (App.js of the suggested sandbox):

import React from "react";
import {
  SplitterElement,
  SplitterLayout,
  AnalyticalTable,
  ThemeProvider,
  ShellBar
} from "@ui5/webcomponents-react";

export default function App() {
  const table = (
    <AnalyticalTable
      data={[
        {
          name: "Germany",
          path: ["Germany"],
          country: undefined,
          description: undefined
        },
        {
          name: "France",
          path: ["France"],
          country: undefined,
          description: undefined
        }
      ]}
      columns={[
        { id: "name", accessor: "name", Header: "ID" },
        { id: "description", accessor: "description", Header: "Description" }
      ]}
      subRowsKey={"children"}
      style={{
        inlineSize: "100%",
        blockSize: "100%"
      }}
    />
  );

  return (
    <ThemeProvider>
      <ShellBar primaryTitle="UI5 Web Components for React Issue Template" />
      <SplitterLayout
        style={{
          inlineSize: "100%",
          blockSize: "100%"
        }}
      >
        <SplitterElement size="70%">{table}</SplitterElement>
        <SplitterElement size="30%">
          <div style={{ width: "100%" }} />
        </SplitterElement>
      </SplitterLayout>
    </ThemeProvider>
  );
}

To Reproduce Steps to reproduce the behavior: Just move the slider. Heads up though, I wasn't able to ever get rid of the scrollbar in the sandbox. In storybook, it will randomly appear or disappear as I move the slider, let go, and the table resizes.

Expected behavior The expected behavior is to not have a horizontal scrollbar as the table fits neatly into its surrounding container.

Screenshots tableheaderrow tableheader

UI5 Web Components for React Information @ui5/webcomponents version: "1.4.0" @ui5/webcomponents-react version: "0.23.2" Operating System: Win10 Browser: Chrome

Thanks a lot in advance!

ui5-webcomponents-react-bot commented 2 years ago

:tada: This issue has been resolved in version v0.28.0 :tada:

The release is available on v0.28.0

Your semantic-release bot :package::rocket:

ej612 commented 2 years ago

Hi there,

I really hate to be the bearer of bad news, but it seems that the bug is still there. On storybook, we observe it arbitrarily, but I was able to reproduce it more systematically in this sandbox.

If the sandbox is unavailable, here is App.js:

import React from "react";
import {
  SplitterElement,
  SplitterLayout,
  AnalyticalTable,
  ThemeProvider,
  IllustratedMessage,
  ShellBar
} from "@ui5/webcomponents-react";
import "@ui5/webcomponents-fiori/dist/illustrations/EmptyList.js";

export default function App() {
  const table = (
    <AnalyticalTable
      data={[
        {
          name: "Germany",
          path: ["Germany"],
          country: undefined,
          description: undefined
        },
        {
          name: "France",
          path: ["France"],
          country: undefined,
          description: undefined
        }
      ]}
      columns={[
        { id: "name", accessor: "name", Header: "ID" },
        { id: "description", accessor: "description", Header: "Description" }
      ]}
      subRowsKey={"children"}
      style={{
        inlineSize: "100%",
        blockSize: "100%"
      }}
    />
  );

  return (
    <ThemeProvider>
      <ShellBar primaryTitle="UI5 Web Components for React Issue Template" />
      <SplitterLayout
        style={{
          inlineSize: "100%",
          blockSize: "100%"
        }}
      >
        <SplitterElement size="70%">{table}</SplitterElement>
        <SplitterElement size="30%">
          <IllustratedMessage
            name="EmptyList"
            titleText={"titleText"}
            subtitleText={"subtitleText"}
          />
        </SplitterElement>
      </SplitterLayout>
    </ThemeProvider>
  );
}

Thanks in advance!

Lukas742 commented 2 years ago

Hi @ej612

your codeSandbox is using 0.27.x of @ui5/webcomponents-react, which does not include the fix. (I clicked the wrong codeSandbox example link from the initial post. Sorry about that ;) )

I tried reproducing with the latest version but wasn't able to (see video). Could you please check if the version was causing this issue? If an unnecessary scrollbar is still rendered even with the newest version for you, could you please let us know your screen resolution of the codeSandbox window when the error occurs (preferably without iframe - for this you can click on the button on the top right of the codesandbox browser window), your OS and browser you are using?

Edit: You can test it with this codeSandbox

https://user-images.githubusercontent.com/9749730/192543770-c45a34dc-8d0b-4cd8-b61e-04dc7d951367.mp4

ej612 commented 2 years ago

Hi @Lukas742 Thanks a lot for your response :) I'm still seeing the problem with the new version. I've attached 2 screenshots. In the side-by-side view, I'm seeing it without any modification at all. Just loading the page immediately displays the problem. Capture Capture2

If I pop out the preview window as you suggested, I initially don't see the problem anymore. I was however able to reproduce it with Chrome's "emulating devices" view and setting a resolution of 401x349.

Lukas742 commented 1 year ago

Fixed by #4899

github-actions[bot] commented 1 year ago

:tada: This issue has been resolved in version v1.20.0 :tada:

The release is available on v1.20.0

Your semantic-release bot :package::rocket:

ej612 commented 1 year ago

Hello there, I'm sorry for unearthing this issue again, but it seems the bug is still there on 1.21.0:

image

Here is a sandbox.

Lukas742 commented 1 year ago

Hi @ej612

I could not reproduce the issue. At what resolution does this occur and what browser and OS are you using?

https://github.com/SAP/ui5-webcomponents-react/assets/9749730/08f77bb7-1f6d-40b6-b9fa-0b4991e1a024

ej612 commented 1 year ago

Hi there, I don't have the issue when I'm connected to an external display, I only see it on my laptop's internal display: Win11 Chrome 118 3840x2160 @ 250% Scale. If I change the scale to 100%, the problem is gone. (I'm talking about Windows' scaling, not the browser's) The issue is either there right from the start or not at all, I didn't manage to make it appear or go away by moving the slider. If there's anything I can help you with, please don't hesitate. Thanks!

Lukas742 commented 12 months ago

Hi @ej612

this sounds like a subpixel problem to me. Unfortunately I don't think that there is a way to fix this as it can differ from browser to browser and monitor to monitor. In short it all comes down to rounding issues of our column size calculation, but even with our workaround (using getBoundingClientRect instead of offsetWidth) we can't prevent this in all cases.

ej612 commented 11 months ago

Hi @Lukas742

I spent some more time looking into this issue and was able to narrow it down: The problem occurs when the width of the AnalyticalTable is not a whole number. In this case, the table will round its scrollWidth to the nearest integer, making the scrollbar appear: image

This may or may not be related to the fact that the rows of the table also seem to have their width rounded up, as I described in my opening post.

I have created a sandbox that should allow you to reproduce the issue.

ej612 commented 11 months ago

Hi @Lukas742

Update: The AnalyticalTable seems to be generally allergic to dimensions that are not whole numbers. I managed to trap it in an infinite loop when given the right dimensions. I made a sandbox.

In case you cannot reproduce the problem, here's a video of the bug:

https://github.com/SAP/ui5-webcomponents-react/assets/110408767/55490b9d-33e8-4f45-91b4-33afc4c616fe

Lukas742 commented 11 months ago

Hi @ej612

I spent some more time looking into this issue and was able to narrow it down: The problem occurs when the width of the AnalyticalTable is not a whole number. In this case, the table will round its scrollWidth to the nearest integer, making the scrollbar appear: [...] This may or may not be related to the fact that the rows of the table also seem to have their width rounded up, as I described in my opening post.

I may have found a fix and created a snapshot release (0.0.0-05297a79b2) that includes it. Could you please test if this solves the issue on your end as well? Please note, that when the page is scaled (scaleXFactor is set and transform: scale(x) is applied), there most probably still will be an unnecessary scrollbar visible in some cases.

Update: The AnalyticalTable seems to be generally allergic to dimensions that are not whole numbers. I managed to trap it in an infinite loop when given the right dimensions. I made a sandbox.

This looks like a different bug to me. Unfortunately I was again not able to reproduce it... Could you create another issue for this bug and also include the sandbox, the browser you're using, the resolution of your monitor and OS.

ej612 commented 11 months ago

Hi @Lukas742

Thank you very much for your response and the fix, it seems to work! In the above sandbox, I can see a scrollbar with 1.21.0, but not with 0.0.0-05297a79b2. I'll create another issue for the flickering vertical scrollbar.

Thanks heaps!

Lukas742 commented 11 months ago

Perfect! We'll publish the fix with our next release then. :)

ui5-webcomponents-react-bot commented 10 months ago

:tada: This issue has been resolved in version v1.23.2 :tada:

The release is available on v1.23.2

Your semantic-release bot :package::rocket: