Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Popover: Controlled height based on size of contents #2562

Merged
merged 1 commit into from
Jan 5, 2023

Conversation

EduhCosta
Copy link
Contributor

@EduhCosta EduhCosta commented Dec 12, 2022

Summary

This change handle and fix the following error, reported to us by this thread:
"When we have more content inner dropdown children than the viewport height supports and the anchor is positioned on second half (bottom half) of screen, the initial elements is not accessible"

The images below provide you the bug view:

Screen Shot 2022-07-27 at 1 28 10 PM

BUG's reproduction: https://codesandbox.io/s/dropdown-size-error-3jo09u

Gestalt component involved

  • Popover
  • Dropdown
  • Content

Solution

We fix the max height of children's Popover content to 90% of the viewport's vertical size. If the viewport contains a ScrollBoundaryContainer component, we use the height of this component to set the max height of popover content. In other words, the Popover max size is 90% screen height or 90% of ScrollBoundaryContainer height.

The second solution part is "top positioning" If we have a negative top value, which means if the Controller sets the top with a negative value (< 0), the popover commonly will not allow access to part of its internal content. In front of this case, we set the top's value as 5% of the height available (following the height decisions explained above).

More about Controller and Popover structure here

To understand better each concept, please read the comments attached on code.

How to test it?

  1. Access our current branch deploy: https://deploy-preview-2562--gestalt.netlify.app/
  2. Go to Dropdown page: /web/dropdown
  3. Resizes your screen to position the first example button to the second half of screen, like the image below:

Example of screen size

4. Expand the code example; 5. Use one the following codes to test the solution:

Example 01

function Example() {
  const [open, setOpen] = React.useState(false);
  const [selected, setSelected] = React.useState(null);
  const anchorRef = React.useRef(null);
  const onSelect = ({ item }) => setSelected(item);

  const items = new Array(100).fill({ name: "item" }).map((item, index) => (
    <Dropdown.Item
      key={item.name + index}
      onSelect={onSelect}
      option={{ value: 'item ' + index, label: 'Item ' + index }}
      selected={selected}
    />
  ));

  return (
    <Flex height="100%" justifyContent="center">
      <Button
        accessibilityControls="demo-dropdown-example"
        accessibilityExpanded={open}
        accessibilityHaspopup
        iconEnd="arrow-down"
        onClick={() => setOpen((prevVal) => !prevVal)}
        ref={anchorRef}
        selected={open}
        size="lg"
        text="Menu"
      />
      {open && (
        <Dropdown
          anchor={anchorRef.current}
          id="demo-dropdown-example"
          onDismiss={() => setOpen(false)}
        >
        {items}
        </Dropdown>
      )}
    </Flex>
  );
}

Example 02

function Example() {
  const [open, setOpen] = React.useState(false);
  const [selected, setSelected] = React.useState(null);
  const anchorRef = React.useRef(null);
  const onSelect = ({ item }) => setSelected(item);

  const items = new Array(100).fill({ name: "item" }).map((item, index) => (
    <Dropdown.Item
      key={item.name + index}
      onSelect={onSelect}
      option={{ value: 'item ' + index, label: 'Item ' + index }}
      selected={selected}
    />
  ));

  return (
    <Flex height="100%" width="100%" justifyContent="center">
      <Box width="100%">
        <ScrollBoundaryContainer height={200}>
          <Button
            accessibilityControls="demo-dropdown-example"
            accessibilityExpanded={open}
            accessibilityHaspopup
            iconEnd="arrow-down"
            onClick={() => setOpen((prevVal) => !prevVal)}
            ref={anchorRef}
            selected={open}
            size="lg"
            text="Menu"
          />
            {open && (
              <Dropdown
                anchor={anchorRef.current}
                id="demo-dropdown-example"
                onDismiss={() => setOpen(false)}
              >
              {items}
              </Dropdown>
            )}
        </ScrollBoundaryContainer>
      </Box>
    </Flex>
  );
}

Links

@EduhCosta EduhCosta requested a review from a team as a code owner December 12, 2022 19:42
@netlify
Copy link

netlify bot commented Dec 12, 2022

Deploy Preview for gestalt ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit ac032dd
🔍 Latest deploy log https://app.netlify.com/sites/gestalt/deploys/63b6d6c8853abf0008561b92
😎 Deploy Preview https://deploy-preview-2562--gestalt.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@EduhCosta EduhCosta added bug Something isn't working patch release Patch release labels Dec 12, 2022
@dangerismycat dangerismycat removed the bug Something isn't working label Dec 15, 2022
@dangerismycat
Copy link
Contributor

I think we're going to need a better way to test this fix. I don't see a noticeable difference from prod using the examples provided. Here's a comparison of prod and this PR using the first example you suggested:

Screen.Recording.2022-12-14.at.4.39.30.PM.mov

Copy link
Contributor

@dangerismycat dangerismycat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great! Thanks for the detailed summary and the test cases. Nice work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
patch release Patch release
Projects
None yet
2 participants