feat(toolbar): handle toolbar overflow

packages/example-app/src/VersionControl/Changes/ChangesView/ChangesViewToolbar.tsx

@@ -15,7 +15,7 @@ import { VcsActionIds } from "../../VcsActionIds";
 
 export function ChangesViewToolbar() {
   return (
-    <Toolbar hasBorder>
+    <Toolbar border="bottom">
       <ActionButton actionId={VcsActionIds.REFRESH} />
       <ActionButton actionId={VcsActionIds.ROLLBACK} />
       <ActionButton actionId={VcsActionIds.SHOW_DIFF} />

packages/jui/src/Icon/useSvgIcon.tsx

@@ -35,11 +35,10 @@ export function useSvgIcon(
       if (svg) {
         if (!unmounted && ref?.current) {
           if (ref) {
-            // potential SSR issues here?
             ref.current?.querySelector("svg")?.remove();
             const svgElement = document.createElement("svg");
             ref.current?.appendChild(svgElement);
-            svgElement.outerHTML = svg;
+            svgElement.outerHTML = makeIdsUnique(svg); // UNSAFE! Would require sanitization, or icon sources must be trusted.
             delete ref.current?.dataset.loadingIcon;
           }
         }
@@ -53,3 +52,30 @@ export function useSvgIcon(
     };
   }, [path, selected]);
 }
+
+/**
+ * If multiple instance of the same icon is rendered at the same time, and the SVG includes
+ * url(#...) references to locally defined ids, in some cases the icon is not rendered properly.
+ * because of ids colliding. We make sure the ids are unique in each rendered icon.
+ */
+function makeIdsUnique(svg: string): string {
+  const randomPostfix = (Math.random() * 1000).toFixed(0);
+  const idMatches = svg.matchAll(/id="(.*?)"/g);
+  return [...idMatches].reduce((modifiedSvg, [_, id]) => {
+    const newId = `${id}-${randomPostfix}`;
+    return replaceAll(
+      `id="${id}"`,
+      `id="${newId}"`,
+      replaceAll(`url(#${id})`, `url(#${newId})`, modifiedSvg)
+    );
+  }, svg);
+}
+
+function replaceAll(theOld: string, theNew: string, str: string): string {
+  const replaced = str.replace(theOld, theNew);
+  const replacedAgain = replaced.replace(theOld, theNew);
+  if (replaced === replacedAgain) {
+    return replaced;
+  }
+  return replaceAll(theOld, theNew, replacedAgain);
+}

packages/jui/src/InputField/InputField.stories.tsx

@@ -1,5 +1,5 @@
-import { Meta, StoryFn, StoryObj } from "@storybook/react";
 import React from "react";
+import { Meta, StoryObj } from "@storybook/react";
 import { InputField, InputFieldProps } from "./InputField";
 
 export default {

packages/jui/src/Menu/Menu.stories.tsx

@@ -260,7 +260,7 @@ export const ContextMenu: StoryObj<{
                 />
               </Item>
               <Divider />
-              <Item textValue="Go to">
+              <Item title="Go to">
                 <Item textValue="Navigation Bar">
                   <MenuItemLayout content="Navigation Bar" shortcut="`⌘↑`" />
                 </Item>

packages/jui/src/Menu/MenuTrigger.tsx

@@ -1,4 +1,4 @@
-import React, { HTMLProps, RefObject } from "react";
+import React, { HTMLAttributes, RefObject } from "react";
 import { useButton } from "@react-aria/button";
 import { useMenuTrigger } from "@react-aria/menu";
 import { useOverlay, useOverlayPosition } from "@react-aria/overlays";
@@ -7,14 +7,15 @@ import { useMenuTriggerState } from "@react-stately/menu";
 import { MenuTriggerProps as AriaMenuTriggerProps } from "@react-types/menu";
 
 import { MenuOverlay } from "./MenuOverlay";
+import { AriaButtonProps } from "@react-types/button";
 
 export interface MenuTriggerProps
   extends Omit<AriaMenuTriggerProps, "closeOnSelect"> {
   restoreFocus?: boolean;
   // TODO: replace render function children with normal children, and utilize PressResponder. Add a story for the
   //  edge case of custom trigger, using PressResponder
   children: (
-    props: HTMLProps<HTMLElement>,
+    props: HTMLAttributes<HTMLButtonElement>,
     ref: RefObject<any> // Using a generic didn't seem to work for some reason
   ) => React.ReactNode;
   // NOTE: there is a chance of unchecked breaking change here, since this is not explicitly mentioned as public API
@@ -60,14 +61,12 @@ export const MenuTrigger: React.FC<MenuTriggerProps> = ({
     state,
     triggerRef
   );
-  const { buttonProps } = useButton(
-    {
-      ...triggerProps,
-      // @ts-expect-error: preventFocusOnPress is not defined in public API of useButton
-      preventFocusOnPress,
-    },
-    triggerRef
-  );
+  const ariaButtonProps: AriaButtonProps<"button"> = {
+    ...triggerProps,
+    // @ts-expect-error: preventFocusOnPress is not defined in public API of useButton
+    preventFocusOnPress,
+  };
+  const { buttonProps } = useButton(ariaButtonProps, triggerRef);
   const { overlayProps } = useOverlay(
     {
       onClose: () => {

packages/jui/src/Popup/Popup.tsx

@@ -26,7 +26,7 @@ import { PopupContext, PopupControllerContext } from "./PopupContext";
 import { PopupLayout } from "./PopupLayout";
 import { StyledPopupHint } from "./StyledPopupHint";
 
-const StyledPopupContainer = styled.div`
+export const StyledPopupContainer = styled.div`
   position: fixed;
   box-sizing: border-box;
   // not checked if there should be a better substitute for * in the following colors. Maybe "Component"?
@@ -175,6 +175,7 @@ export const _Popup = (
   return (
     <Overlay>
       <OverlayInteractionHandler {...overlayInteractionHandlerProps}>
+        {/* TODO: FocusScope is redundant. Test focus restoration without it (in status bar progress), and remove it if unnecessary */}
         <FocusScope restoreFocus>
           <StyledPopupContainer
             ref={ref}

packages/jui/src/StyledSeparator.tsx

@@ -15,7 +15,7 @@ const DarculaSeparatorUI: SeparatorUI = {
 const defaultSize =
   2 * DarculaSeparatorUI.STRIPE_INDENT + DarculaSeparatorUI.STRIPE_WIDTH;
 
-const StyledSeparator = styled.hr(({ theme }) => ({
+export const StyledSeparator = styled.hr(({ theme }) => ({
   backgroundColor: theme.color(
     "Separator.separatorColor",
     theme.dark ? "#cdcdcd" : "#515151"

packages/jui/src/Tabs/2-Tabs.stories.tsx

@@ -1,6 +1,6 @@
 import { Meta, StoryObj } from "@storybook/react";
 import { MenuItemLayout, PlatformIcon } from "@intellij-platform/core";
-import React from "react";
+import React, { useEffect, useState } from "react";
 import { TabContentLayout, TabItem, TabsProps } from ".";
 import { Tabs } from "./Tabs";
 import { DOMProps } from "@react-types/shared";
@@ -53,20 +53,22 @@ export const DynamicItems: StoryObj<StoryProps> = {
 
 export const Overflow: StoryObj<StoryProps & { maxWidth: number }> = {
   render: ({ maxWidth = 800, ...props }) => {
-    const tabs = Array(10)
-      .fill(null)
-      .map((_, index) => ({
-        title: `Big tab title #${index}`,
-        icon: "nodes/folder",
-      }));
+    const [tabs, setTabs] = useState(
+      Array(10)
+        .fill(null)
+        .map((_, index) => ({
+          title: `Big tab title #${index}`,
+          icon: "nodes/folder",
+        }))
+    );
     return (
       <div style={{ maxWidth }}>
         <Tabs {...props} items={tabs}>
           {(tab) => {
             const icon = <PlatformIcon icon={tab.icon} />;
             return (
               <TabItem
-                key={tab.title}
+                key={tabs.indexOf(tab)}
                 textValue={tab.title}
                 inOverflowMenu={
                   <MenuItemLayout content={tab.title} icon={icon} />
@@ -77,6 +79,45 @@ export const Overflow: StoryObj<StoryProps & { maxWidth: number }> = {
             );
           }}
         </Tabs>
+        <div style={{ marginTop: "1rem" }}>
+          <button
+            onClick={() => {
+              setTabs((tabs) => [
+                ...tabs,
+                {
+                  title: `Big tab title #${tabs.length}`,
+                  icon: "nodes/folder",
+                },
+              ]);
+            }}
+          >
+            Add tab
+          </button>
+          <button
+            onClick={() => {
+              setTabs((tabs) =>
+                tabs.map((tab, index) => {
+                  if (index === 4) {
+                    const title = tab.title;
+                    return {
+                      ...tab,
+                      title:
+                        title.replace(/ \(.*\)/, "") +
+                        ` ( edited - ${
+                          title.includes("short")
+                            ? "long long long long"
+                            : "short"
+                        })`,
+                    };
+                  }
+                  return tab;
+                })
+              );
+            }}
+          >
+            Change title of tab #4
+          </button>
+        </div>
       </div>
     );
   },

packages/jui/src/Tabs/Tab.tsx

@@ -2,12 +2,12 @@ import { useTab } from "@react-aria/tabs";
 import { TabListState } from "@react-stately/tabs";
 import { Node } from "@react-types/shared";
 import { StyledDefaultTab } from "./StyledDefaultTab";
-import React, { useEffect } from "react";
+import React, { ForwardedRef, forwardRef, RefObject, useEffect } from "react";
+import useForwardedRef from "@intellij-platform/core/utils/useForwardedRef";
 
 type TabProps<T extends object> = {
   state: TabListState<object>;
   item: Node<T>;
-  intersectionObserver: IntersectionObserver | null;
   /**
    * {@see TabsProps#focusable}
    */
@@ -19,16 +19,12 @@ type TabProps<T extends object> = {
   Component?: typeof StyledDefaultTab;
 };
 
-export const Tab = <T extends object>({
-  state,
-  item,
-  focusable,
-  active,
-  Component = StyledDefaultTab,
-  intersectionObserver,
-}: TabProps<T>): React.ReactElement => {
+export const Tab = forwardRef(function Tab<T extends object>(
+  { state, item, focusable, active, Component = StyledDefaultTab }: TabProps<T>,
+  forwardedRef: ForwardedRef<HTMLDivElement>
+): React.ReactElement {
   const { key, rendered } = item;
-  const ref = React.useRef(null);
+  const ref = useForwardedRef(forwardedRef);
   const {
     tabProps: {
       /**
@@ -41,7 +37,6 @@ export const Tab = <T extends object>({
   } = useTab({ key }, state, ref);
   const isSelected = state.selectedKey === key;
   const isDisabled = state.disabledKeys.has(key);
-  useIntersectionObserver(ref, intersectionObserver);
 
   return (
     <Component
@@ -55,19 +50,4 @@ export const Tab = <T extends object>({
       {rendered}
     </Component>
   );
-};
-
-function useIntersectionObserver(
-  ref: React.MutableRefObject<null>,
-  intersectionObserver: IntersectionObserver | null
-) {
-  useEffect(() => {
-    const element = ref.current;
-    if (element) {
-      intersectionObserver?.observe(element);
-      return () => {
-        intersectionObserver?.unobserve(element);
-      };
-    }
-  }, [intersectionObserver]);
-}
+});

packages/jui/src/Tabs/Tabs.tsx

@@ -5,9 +5,10 @@ import { useTabListState } from "@react-stately/tabs";
 import { AriaTabListProps } from "@react-types/tabs";
 import { StyledHorizontalOverflowShadows } from "./StyledHorizontalOverflowShadows";
 import { TabsOverflowMenu } from "./TabsOverflowMenu";
-import { useCollectionOverflowObserver } from "./useCollectionOverflowObserver";
+import { useOverflowObserver } from "../utils/overflow-utils/useOverflowObserver";
 import { useHasOverflow } from "./useHasOverflow";
-import { styled, css } from "@intellij-platform/core/styled";
+import { css, styled } from "@intellij-platform/core/styled";
+import { notNull } from "@intellij-platform/core/utils/array-utils";
 import { StyledDefaultTab } from "./StyledDefaultTab";
 import { StyledDefaultTabs } from "./StyledDefaultTabs";
 import { Tab } from "./Tab";
@@ -120,8 +121,14 @@ export const Tabs = <T extends object>({
   const { tabListProps } = useTabList(props, state, ref);
 
   const { scrolledIndicatorProps, hasOverflow } = useHasOverflow({ ref });
-  const { overflowedKeys, intersectionObserver } =
-    useCollectionOverflowObserver(ref);
+  const { overflowedElements } = useOverflowObserver(ref);
+  const overflowedKeys = new Set(
+    overflowedElements
+      .map((element) =>
+        element instanceof HTMLElement ? element.dataset["key"] : null
+      )
+      .filter(notNull)
+  );
 
   useEffect(() => {
     if (!noScroll) {
@@ -162,7 +169,6 @@ export const Tabs = <T extends object>({
               focusable={focusable}
               active={active}
               Component={TabComponent}
-              intersectionObserver={intersectionObserver}
             />
           ))}
         </StyledTabList>