Fixes

jpmorganchase · Oct 8, 2024 · bd00f64 · bd00f64
1 parent 5dcc8ce
commit bd00f64
Show file tree

Hide file tree

Showing 10 changed files with 38 additions and 24 deletions.
diff --git a/.changeset/grumpy-lies-bathe.md b/.changeset/grumpy-lies-bathe.md
@@ -6,6 +6,6 @@ Added SemanticIconProvider to provide a mechanism to swap out built-in component
 
 ```tsx
 <SemanticIconProvider iconMap={iconMap}>
-  <StoryFn />
+  <App />
 </SemanticIconProvider>
 ```
diff --git a/packages/core/src/__tests__/__e2e__/link/Link.cy.tsx b/packages/core/src/__tests__/__e2e__/link/Link.cy.tsx
@@ -42,17 +42,4 @@ describe("GIVEN a link", () => {
 
     cy.findByTestId(/TearOutIcon/i).should("not.exist");
   });
-
-  it('WHEN target is "_blank" AND IconComponent is provided, THEN should render the provided IconComponent', () => {
-    const CustomIcon = () => <div data-testid="CustomIcon">Custom Icon</div>;
-
-    cy.mount(
-      <Link href="#root" target="_blank" IconComponent={CustomIcon}>
-        Action
-      </Link>,
-    );
-
-    cy.findByTestId(/CustomIcon/i).should("exist");
-    cy.findByTestId(/TearOutIcon/i).should("not.exist");
-  });
 });
diff --git a/packages/core/src/__tests__/__e2e__/semantic-icon-provider/SemanticIconProvider.cy.tsx b/packages/core/src/__tests__/__e2e__/semantic-icon-provider/SemanticIconProvider.cy.tsx
@@ -22,7 +22,7 @@ const TestComponent = () => {
   );
 };
 
-describe("SemanticIconProvider Tests", () => {
+describe("SemanticIconProvider", () => {
   it("should use default icons when provider is not wrapped", () => {
     cy.mount(<TestComponent />);
     cy.get('[data-testid="ChevronDownIcon"]').should("exist");
@@ -31,7 +31,7 @@ describe("SemanticIconProvider Tests", () => {
     cy.get('[data-testid="UserSolidIcon"]').should("exist");
   });
 
-  it("should override only specific icons when provider is partially wrapped", () => {
+  it("should support overriding only specific icons", () => {
     cy.mount(
       <SemanticIconProvider
         iconMap={{
@@ -59,7 +59,7 @@ describe("SemanticIconProvider Tests", () => {
     cy.get('[data-testid="UserSolidIcon"]').should("exist");
   });
 
-  it("should override all icons when provider is fully wrapped", () => {
+  it("should support overriding all icons", () => {
     cy.mount(
       <SemanticIconProvider
         iconMap={{

diff --git a/packages/core/src/avatar/Avatar.tsx b/packages/core/src/avatar/Avatar.tsx
@@ -61,9 +61,12 @@ export const Avatar = forwardRef<HTMLDivElement, AvatarProps>(function Avatar(
   const targetWindow = useWindow();
   const { UserIcon } = useIcon();
 
-  const fallbackIcon = fallbackIconProp || (
-    <UserIcon aria-label="User Avatar" />
-  );
+  const fallbackIcon =
+    fallbackIconProp === undefined ? (
+      <UserIcon aria-label="User Avatar" />
+    ) : (
+      fallbackIconProp
+    );
 
   useComponentCssInjection({
     testId: "salt-avatar",

diff --git a/packages/core/stories/avatar/avatar.qa.stories.tsx b/packages/core/stories/avatar/avatar.qa.stories.tsx
@@ -1,4 +1,5 @@
 import { Avatar } from "@salt-ds/core";
+import { SaltShakerIcon } from "@salt-ds/icons";
 import type { Meta, StoryFn } from "@storybook/react";
 import {
   QAContainer,
@@ -18,6 +19,7 @@ export const AllVariantsGrid: StoryFn<QAContainerProps> = (props) => (
     <Avatar size={1} name="Alex Brailescu" src={persona1 as string} />
     <Avatar size={2} src="bad_url" name="Peter Piper" />
     <Avatar size={3} src="bad_url" />
+    <Avatar size={3} fallbackIcon={<SaltShakerIcon />} />
   </QAContainer>
 );
 
@@ -45,6 +47,7 @@ export const NoStyleInjectionGrid: StoryFn<QAContainerNoStyleInjectionProps> = (
     <Avatar size={1} name="Alex Brailescu" src={persona1 as string} />
     <Avatar size={2} src="bad_url" name="Peter Piper" />
     <Avatar size={3} src="bad_url" />
+    <Avatar size={3} fallbackIcon={<SaltShakerIcon />} />
   </QAContainerNoStyleInjection>
 );
 

diff --git a/packages/core/stories/link/link.qa.stories.tsx b/packages/core/stories/link/link.qa.stories.tsx
@@ -1,4 +1,5 @@
 import { Link } from "@salt-ds/core";
+import { UserIcon } from "@salt-ds/icons";
 import type { Meta, StoryFn } from "@storybook/react";
 import {
   QAContainer,
@@ -40,6 +41,16 @@ export const AllVariantsGrid: StoryFn<QAContainerProps> = (props) => (
     >
       Forced visited
     </Link>
+    <Link
+      href="https://www.google.com"
+      target="_blank"
+      IconComponent={UserIcon}
+    >
+      Custom icon
+    </Link>
+    <Link href="https://www.google.com" target="_blank" IconComponent={null}>
+      No icon
+    </Link>
   </QAContainer>
 );
 
@@ -71,6 +82,16 @@ export const NoStyleInjectionGrid: StoryFn<QAContainerNoStyleInjectionProps> = (
         <strong>Strong</strong> and <small>small</small> truncation example
       </Link>
     </div>
+    <Link
+      href="https://www.google.com"
+      target="_blank"
+      IconComponent={UserIcon}
+    >
+      Custom icon
+    </Link>
+    <Link href="https://www.google.com" target="_blank" IconComponent={null}>
+      No icon
+    </Link>
   </QAContainerNoStyleInjection>
 );
 

diff --git a/packages/lab/src/breadcrumbs/internal/BreadcrumbsCollapsed.tsx b/packages/lab/src/breadcrumbs/internal/BreadcrumbsCollapsed.tsx
@@ -34,7 +34,7 @@ export const BreadcrumbsCollapsed = ({
     return "";
   });
 
-  const { OverflowIcon } = useIcon() || {};
+  const { OverflowIcon } = useIcon();
   const key = keys ? keys.join("") : "";
   const { ref, shouldFocusOnMount } =
     useFocusMenuRemount<HTMLButtonElement>(key);

diff --git a/packages/lab/src/dropdown/DropdownButton.tsx b/packages/lab/src/dropdown/DropdownButton.tsx
@@ -87,7 +87,7 @@ export const DropdownButton = forwardRef(function DropdownButton(
     window: targetWindow,
   });
   const { ExpandIcon } = useIcon();
-  const Icon = IconComponent ?? ExpandIcon;
+  const Icon = IconComponent === undefined ? ExpandIcon : IconComponent;
   const { inFormField } = useFormFieldLegacyProps();
   // FIXME: use polymorphic button
   // We don't want the 'button' tag to be shown in the DOM to trigger some accessibility testing

diff --git a/packages/lab/src/query-input/internal/ValueSelector.tsx b/packages/lab/src/query-input/internal/ValueSelector.tsx
@@ -60,7 +60,7 @@ export function ValueSelector(props: ValueSelectorProps) {
     setHighlightedCategoryIndex,
     setHighlightedIndex,
   } = props;
-  console.log("Testtttttt");
+
   const selectedCategoryValues = useMemo(() => {
     if (!selectedCategory) {
       return [];

diff --git a/packages/lab/src/tabs-next/OverflowMenu.tsx b/packages/lab/src/tabs-next/OverflowMenu.tsx
@@ -18,7 +18,7 @@ export const OverflowMenu = forwardRef<HTMLDivElement, OverflowMenuProps>(
     const { tabs, ...rest } = props;
     const { ref, overflowCount, isOverflowing } =
       useOverflowMenu<HTMLDivElement>();
-    const { OverflowIcon } = useIcon() || {};
+    const { OverflowIcon } = useIcon();
     const handleRef = useForkRef(ref, forwardedRef);
     const itemVisibility = useOverflowContext(
       (context) => context.itemVisibility,