Skip to content

Commit

Permalink
Merge pull request #10660 from liamdebeasi/ld/dropdown-render
Browse files Browse the repository at this point in the history
fix(popover): dropdown does not flicker on firefox
  • Loading branch information
bhollis authored Aug 8, 2024
2 parents fd6d32a + 8b0bf9e commit fab6f0c
Show file tree
Hide file tree
Showing 5 changed files with 81 additions and 49 deletions.
1 change: 1 addition & 0 deletions docs/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
## Next

* Fixed character sorting on the Loadouts page.
* Dropdown no longer flickers on Firefox.

## 8.31.0 <span class="changelog-date">(2024-08-04)</span>

Expand Down
17 changes: 10 additions & 7 deletions src/app/dim-ui/Dropdown.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -74,13 +74,16 @@ export default function Dropdown({
const buttonRef = useRef<HTMLButtonElement>(null);
const menuRef = useRef<HTMLDivElement>(null);

usePopper({
contents: menuRef,
reference: buttonRef,
placement,
offset,
fixed,
});
usePopper(
{
contents: menuRef,
reference: buttonRef,
placement,
offset,
fixed,
},
[isOpen, items],
);

return (
<div className={className}>
Expand Down
15 changes: 9 additions & 6 deletions src/app/dim-ui/PressTip.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -93,12 +93,15 @@ function Control({
const pressTipRoot = useContext(PressTipRoot);
const [customization, customizeTooltip] = useState<TooltipCustomization>({ className: null });

usePopper({
contents: tooltipContents,
reference: triggerRef,
arrowClassName: styles.arrow,
placement,
});
usePopper(
{
contents: tooltipContents,
reference: triggerRef,
arrowClassName: styles.arrow,
placement,
},
[open],
);

if (!tooltip) {
const { style } = rest;
Expand Down
15 changes: 9 additions & 6 deletions src/app/dim-ui/Select.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -79,12 +79,15 @@ export default function Select<T>({
);
const [dropdownHeight, setDropdownHeight] = useState<number | undefined>();

usePopper({
contents: menuRef,
reference: buttonRef,
placement: 'bottom-start',
offset: 2,
});
usePopper(
{
contents: menuRef,
reference: buttonRef,
placement: 'bottom-start',
offset: 2,
},
[isOpen, items],
);

if (!selectedItem) {
throw new Error('value must correspond to one of the provided options');
Expand Down
82 changes: 52 additions & 30 deletions src/app/dim-ui/usePopper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,35 +97,38 @@ const popperOptions = (
};
};

export function usePopper({
contents,
reference,
arrowClassName,
menuClassName,
boundarySelector,
placement,
offset,
fixed,
padding,
}: {
/** A ref to the rendered contents of a popper-positioned item */
contents: React.RefObject<HTMLElement>;
/** An ref to the item that triggered the popper, which anchors it */
reference: React.RefObject<HTMLElement>;
/** A class used to identify the arrow */
arrowClassName?: string;
/** A class used to identify the sidecar menu */
menuClassName?: string;
/** An optional additional selector for a "boundary area" */
boundarySelector?: string;
/** Placement preference of the popper. Defaults to "auto" */
placement?: Placement;
/** Offset of how far from the element to shift the popper. */
offset?: number;
/** Is this placed on a fixed item? Workaround for https://github.com/popperjs/popper-core/issues/1156. TODO: make a "positioning context" context value for this */
fixed?: boolean;
padding?: Padding;
}) {
export function usePopper(
{
contents,
reference,
arrowClassName,
menuClassName,
boundarySelector,
placement,
offset,
fixed,
padding,
}: {
/** A ref to the rendered contents of a popper-positioned item */
contents: React.RefObject<HTMLElement>;
/** An ref to the item that triggered the popper, which anchors it */
reference: React.RefObject<HTMLElement>;
/** A class used to identify the arrow */
arrowClassName?: string;
/** A class used to identify the sidecar menu */
menuClassName?: string;
/** An optional additional selector for a "boundary area" */
boundarySelector?: string;
/** Placement preference of the popper. Defaults to "auto" */
placement?: Placement;
/** Offset of how far from the element to shift the popper. */
offset?: number;
/** Is this placed on a fixed item? Workaround for https://github.com/popperjs/popper-core/issues/1156. TODO: make a "positioning context" context value for this */
fixed?: boolean;
padding?: Padding;
},
deps: React.DependencyList = [],
) {
const popper = useRef<Instance | undefined>();

const destroy = () => {
Expand Down Expand Up @@ -160,5 +163,24 @@ export function usePopper({
}

return destroy;
});
}, [
contents,
reference,
arrowClassName,
menuClassName,
boundarySelector,
placement,
offset,
fixed,
padding,

/**
* Doing ...deps allows us to pass dependencies from the components that rely on
* usePopper. Certain popovers are only shown when specific conditions are met,
* so by making those conditions dependencies we can position the popover
* correctly once the popover is actually shown.
*/
// eslint-disable-next-line react-hooks/exhaustive-deps
...deps,
]);
}

0 comments on commit fab6f0c

Please sign in to comment.