fix: Constrain dropdown width to viewport

[jperals]

Description

After #1423, dropdowns of the autosuggest, select and multiselect components expand beyond the trigger width, like this:

The problem is, that doesn't account for the available space, so in a narrow viewport or in a narrow container with hidden overflow the dropdown will be cut off:

This is fixed here by opting out of that behavior if it was detected that the the dropdown overflows. So if that would happen, it will not stretch beyond the trigger:

Issue: AWSUI-22328

How has this been tested?

Added new test page, which I used for both running manual testing and for writing new integration tests.

Review checklist

The following items are to be evaluated by the author(s) and the reviewer(s).

Correctness

• Changes include appropriate documentation updates.
• Changes are backward-compatible if not indicated, see CONTRIBUTING.md.
• Changes do not include unsupported browser features, see CONTRIBUTING.md.
• Changes were manually tested for accessibility, see accessibility guidelines.

Security

• If the code handles URLs: all URLs are validated through the checkSafeUrl function.

Testing

• Changes are covered with new/existing unit tests?
• Changes are covered with new/existing integration tests?

---

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[codecov]

Codecov Report

Patch coverage: 93.54% and project coverage change: +0.01% 🎉

Comparison is base (8766148) 93.98% compared to head (b2ca3ff) 94.00%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1555      +/-   ##
==========================================
+ Coverage   93.98%   94.00%   +0.01%     
==========================================
  Files         645      645              
  Lines       17387    17402      +15     
  Branches     5708     5712       +4     
==========================================
+ Hits        16342    16358      +16     
+ Misses        976      975       -1     
  Partials       69       69              

Files Changed	Coverage Δ
src/internal/utils/scrollable-containers.ts	89.28% <66.66%> (ø)
...ternal/components/dropdown/dropdown-fit-handler.ts	94.57% <95.65%> (-0.35%)	⬇️
src/internal/components/dropdown/index.tsx	91.30% <100.00%> (+0.22%)	⬆️

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[pan-kot]

Why maxWidth is removed?

[jperals]

Because when stretchBeyondTriggerWidth is true, then both the stretch-beyong-trigger-width class and the maxWidth are applied (see line 106 here) and line 116 here respectively). Here we need to revert both changes.

Why is maxWidth applied separately from the class in the first place? I assume basically because its value comes from JavaScript, here. I guess this could be refactored, e.g, by setting a CSS custom property that could be used in a ruleset under the class selector. This way adding and removing the class would be enough, and this part in particular might be a bit more readable but at the same time this would add a bit more complexity overall, for just removing one line, so not sure it's worth it.

[pan-kot]

I think eventually we need to refactor that entire component but that's not in scope. What I would recommend is to add a code comment to explain why the max-width is removed.

[pan-kot]

What is worrying is that the class is added and removed in different places. There is a chance that with another render of the dropdown the class is going to be re-added but not removed:

function DropdownConsumer () {
  const [counter, setCounter] = useState(0);
  useEffect(() => setInterval(() => setCounter(prev => prev + 1), 1000), []);
  return <Dropdown ... ><LargeContent /></Dropdown>
}

Also, what if the content changes? E.g. initially the dropdown renders a loading state and then the content appears. Does overflow calculation re-apply?

[jperals]

You're right, there is an issue here because this useLayoutEffect hook has some dependencies so that it is called essentially when the dropdown changes from closed to open, but not if its content is changes while it's open. I think we will have to call onDropdownOpen (or at least part of it) essentially on every render. Here are two things to consider:

• Performance: the function call takes 2ms in average (on my machine), so I think this should not be a concern
• Behavior: should the dropdown placement change if its content changes? Since this hook only runs when opening, the current behavior is not to reposition the dropdown in any case while opened. I think this current behavior is good when select options are added or removed (which can happen with Autoselect) to prevent moving content that is already present in the page, but when changing the status from loading to content loaded, I think it would actually make sense to recalculate the position.

[pan-kot]

I agree that the dropdown placement should stay the same but the concern is it might not to. The class that fixes the issue is only removed in the effect so the next render can bring it back and as result break the positioning. This can probably be fixed by applying the class from within the effect as well, wdyt?

[jperals]

The code that (potentially) removes the class (and the maxWidth) is in a useLayoutEffect hook, so it will run after the component renders but before the browser paints the screen. I just moved this code to its own useLayoutEffect hook, because as said, we will have to check this on every render.

[pan-kot]

Is there an advantage in using the layout effect with an empty dependencies array over computing the props in the render cycle, like:

const canOverflow = ...;
const maxWidth = canOverflow ? defaultMaxWidth : undefined;
const overflowClasses = canOverflow ? [styles['stretch-beyond-trigger-width']] : [];

[jperals]

Exactly, we need to know what the rendered dimensions are going to be, so we need to render the component first...

[pan-kot]

I am thinking on a slightly different solution then to not introduce the potential performance degradation. What if when the overflow is banned it is reflected in a new state so that the next renders will not override it?

useLayoutEffect(() => {
  if (!canOverflow) {
    setBanOveflow(true);
  }
}, [...]);

return <div style={{ maxWidth: !banOverflow ? defaultMaxWidth : undefined }} className={clsx(..., !banOverflow && styles['stretch-beyond-trigger-width'])} />

Alternatively, the state can be replaced with e.g. a new class name (like styles['ban-overflow']) to remove the effects in CSS. This class name can be added in the layout effect but then we need to ensure it persists over the next renders which can be achieved by also storing this info in a ref.

[jperals]

Wouldn't the browser still do one paint with the layout that we don't want? Since this would "fix" the next render, but not the current one.

[pan-kot]

Not sure about this one. When the state is changed from the layout effect it probably won't render the previous one. If yes, the alternative approach can be used or the styles and classes can be also removed imperatively.

[pan-kot]

What you can also try is:

useLayoutEffect(() => {
  // ...
  banOverflowRef.current = !canOverflow;
  if (!canOverflow) {
    dropdownRef.classList.add(styles['ban-overflow']);
  } else {
    dropdownRef.classList.remove(styles['ban-overflow']);
  }
  // ...
}, [...]);

return <div ... className={clsx(..., banOverflowRef.current && styles['ban-overflow'])} />;

The above code ensures the styles['ban-overflow'] is always present when we want to ban the overflow. In CSS you can use that class selector to negate the max-width and 'stretch-beyond-trigger-width' effects.