iTwin / appui

Monorepo for iTwin.js AppUi
MIT License
9 stars 2 forks source link

Some popovers overlaps the status bar #1051

Closed vermanas closed 1 month ago

vermanas commented 2 months ago

Describe the bug

Presentation popover used in Design Review app overlaps the status bar when expanded and are partially hidden below the browser window.

To Reproduce

  1. Open Design Review app - contact me for direct link.
  2. Expand presentation widget.
  3. Scroll down to the bottom of it.

Expected Behavior

Presentation popover appears just like the rest of popovers - above the status bar.

Screenshots

image

Desktop (please complete the applicable information)

Additional context

No response

AnDuong249 commented 2 months ago

The problem seems to stem from the 400px default max size that iTwinUI had in their middleware props. I think for this problem, if we can expose the middleware props for the StatusBarPopover, we can adjust the size to our component and fix this issue.

GerardasB commented 1 month ago

I agree, this seems to be a regression introduced by https://github.com/iTwin/iTwinUI/pull/2190 The iTwinUI team suggests making sure that the content is responsive in the popover container while looking for additional ways to mitigate this problem. In case of a Presentation status bar field, it could be: setting a style={{display: "flex"}} to StatusBarPopover. Changing the height: 440px property of .vd-panel-content to min-height: 440px. From the AppUI side, we will forward all Popover props in StatusBarPopover in future releases.

mayank99 commented 1 month ago

I would also recommend using iTwinUI's Surface component. When combined with Surface.Body, it will automatically handle most of the responsive design considerations. You can use this code today (the as prop will work at runtime):

<StatusBarPopover
  // @ts-expect-error: TODO: remove this comment after updating AppUI
  as={Surface}
  content={
    <>
      <Surface.Header>Presentation</Surface.Header>
      <Surface.Body isPadded>
        {/* This will fill the parent and allow scrolling */}
      </Surface.Body>
    </>
  }
>

The middleware.size.availableHeight prop is only there for when you need more control and actually need to set a max-height. Normally you would set max-height in the CSS, but that would get overridden by the size middleware. (iTwinUI could try to read the computed size, but then it wouldn't reflect future updates unless we do multiple passes of forced layout). In most cases, you should not need to set middleware.size.availableHeight. Using this prop does not automatically make your layout responsive, and might hide responsive design issues that are being surfaced right now.