wellyshen/react-cool-virtual

Problem with relative height (100%) container (outerRef)

dehmer opened this issue · 8 comments

First things first: Congratulations on the well designed interface! It was easy as pie to integrate useVirtual() with my existing list. Well done!

Bug Report

Describe the Bug

  • List contains items with variable (but fixed) heights.
  • Container (outerRef) has relative height of 100%. Necessary to scale list height with window height (Electron Application).

How to Reproduce

When scrolling down, at some point the list/container outgrows its allotted size towards to bottom of the screen (see screenshots below). Exactly when list (innerRef) margin-top exceeds container (outerRef) clientHeight.
Also, from this point on, the number of items grows with each step down.

const List = props => {
  const { child, entries } = props
  const { outerRef, innerRef, items, scrollToItem } = useVirtual({
    itemCount: entries.length,
    itemSize: 140, // average/estimate [~100px; ~200px]
    overscanCount: 10 // has no effect on observed issue
  })

  React.useEffect(() => {
    if (props.scroll === 'none') return
    if (props.focusIndex === -1) return
    scrollToItem(props.focusIndex)
  }, [scrollToItem, props.focusIndex, props.scroll])

  const card = ({ index, measureRef }) => {
    const entry = entries[index]
    return child({
      entry,
      id: entry.id, // key
      ref: measureRef,
      focused: props.focusId === entry.id,
      selected: props.selected.includes(entry.id)
    })
  }

  return (
    <div className='list-container' ref={outerRef}>
      <div
        ref={innerRef}
        className='list'
      >
        { entries.length ? items.map(card) : null }
      </div>
    </div>
  )
}

CodeSandbox Link

I can try to isolate the problem, if it would be helpful. I'm holding back until your initial feedback.

Expected Behavior

  • Main issue: List/container should keep its size while scrolling through the complete list.
  • (Minor) issue: Focused item should always be completely visible (scrollToItem(index)).

Screenshots

  • A: first item is focused
  • B: last item is focused before list starts to grow. Focused item is not visible though (right below item 1OSC).
  • C: list has outgrown its allotted height. This should not happen.

Screenshot 2021-08-30 at 14 01 39

Your Environment

  • MacBook Pro (15-inch, 2017)
  • macOS 11.5.2 (Big Sur)
  • Electron 13.1.6
process.versions = {
  "node": "14.16.0",
  "v8": "9.1.269.36-electron.0",
  "uv": "1.40.0",
  "zlib": "1.2.11",
  "brotli": "1.0.9",
  "ares": "1.16.1",
  "modules": "89",
  "nghttp2": "1.41.0",
  "napi": "7",
  "llhttp": "2.1.3",
  "openssl": "1.1.1",
  "icu": "68.1",
  "unicode": "13.0",
  "electron": "13.1.6",
  "chrome": "91.0.4472.124"
}

BTW: For container with absolut height, useVirtual() pretty much works as expected. There are a few minor issue, I will attack later.

And here's the CSS I'm using (I'm not an expert in this field):

.fullscreen {
  position: absolute;
  top: 0;
  left: 0;
  bottom: 0;
  right: 0;
}


/* top level (combined with fullscreen) */
.panel-container {
  z-index: 20;
  pointer-events: none;

  /*
    Give WebKit a chance to calculate explicit height.
    This is necessary to stretch relative sized children to desired height.
  */
  height: calc(100vh - 4em);

  padding-top: 6em;
  padding-bottom: 2em;
  padding-left: 1em;
  padding-right: 1em;

  display: grid;
  grid-gap: 1em;
  grid-template-columns: 26em auto 26em;
  grid-template-rows: auto;
  grid-template-areas: "L . R";
}

.panel-left {
  grid-area: L;
  display: flex;
  flex-direction: column;
}

/* list-container goes to panel-left */

.list-container {
  height: 100%;
  overflow: auto;
}

.list {
  max-height: 0;
  padding-left: 8px;
  padding-right: 8px;
}

@dehmer Could you mind sharing a minimal reproduced code with me through CodeSandbox? In addition, what issues with the fixed container height?

@wellyshen OK, I'll try. Never used CodeSandbox or similar.
The one other issue is, that the last (focused) item in the list is most times not completely visible.
That scrollbar is 'jittering' because of different counts or height estimation is expected and a non-issue for me.

Thanks, and I get back to you as soon as I have something to show on CodeSandbox.

@dehmer Thank you I will check it as soon as the reproduced code is provided.

@wellyshen Wow! CodeSandbox was quite a nice experience.
The result is consistent with the issue described here. Roughly the same behavior for Safari and Chrome (with Electron/Chrome being the target platform).

CodeSandbox: https://codesandbox.io/s/white-glade-xnu3u

Thanks for looking into It!

@dehmer You need to give the style of the outer element a "height" property. Moreover, for dynamic size, we can use measureRef to gradually measure each item when the user is scrolling. Here's the fixed for you.

@wellyshen Thanks you very much! height: inherit does the trick! measureRef I simply forgot and works well too.

I'm closing this issue as fixed.

I have another issue with items array overshooting (more entries than the actual entry list), but I will file another issue for this, when I can reproduce in CodeSandbox.

In the meantime, I try to workaround this issue by returning null in this case. Don't know I it will blow up in my face, but so far it seems to be OK.

const card = ({ index, measureRef }) => {
    const entry = entries[index]
    if (index >= entries.length) {
      console.error('overshooting', `${index}/${entries.length}`)
      return null
    }

    return child({
      entry,
      id: entry.id,
      focused: focusId === entry.id,
      selected: selected.includes(entry.id),
      ref: measureRef
    })
  }

@dehmer No problem mate, thank you for letting this library getting better than better.