Various fixes for errors seen in the console while browsing (metabase…

…#21060)

* shouldNotForwardTransientProp method created for emotion

* BadgeIcon html attributes clean up

* Usage of transient property $hasMargin in Badge for BadgeIcon

* HTML attribute cleanup for LinkRoot

* React key added to ButtonBar child if not defined

* Properties for TableInfoPopover properly typed

* Misc

* analyticsContext property set as non required as non of the parents enforce it or pass a value

* Clean up attributes for SVG in Icon

* CSS fix for share icon special case in Icon

* Checkbox controlled component default value fix

* Misc clean up

* index file for collection side bar

* Collections collectionID type fix

* collection type in NewCollectionItemMenu fix

* Misc

* TokenField fix for value

* noPopover passed properly to WidgetStatusIcon

* Updates to controlled input/checkbox

* VS Code workspace file added to gitignore

* shouldNotForwardTransientProp renamed to shouldForwardNonTransientProp

* AddAggregationButton defaultProps updates

* propTypes for TableInfoPopover added back

.gitignore

@@ -80,3 +80,4 @@ dev/src/dev/nocommit/
 .lsp/*
 !.lsp/config.edn
 .clj-kondo/.cache
+*.code-workspace

frontend/src/metabase/collections/components/CollectionSidebar/Collections/Collections.jsx

@@ -10,7 +10,7 @@ import {
 import { Container } from "./Collections.styled";
 
 const propTypes = {
-  collectionId: PropTypes.number,
+  collectionId: PropTypes.oneOfType([PropTypes.number, PropTypes.string]),
   currentUserId: PropTypes.number,
   handleToggleMobileSidebar: PropTypes.func.isRequired,
   list: PropTypes.array,

frontend/src/metabase/collections/components/NewCollectionItemMenu.jsx

@@ -2,13 +2,13 @@ import React from "react";
 import PropTypes from "prop-types";
 import { t } from "ttag";
 
-import * as Urls from "metabase/lib/urls";
+import { newQuestion, newDashboard, newCollection } from "metabase/lib/urls";
 
 import EntityMenu from "metabase/components/EntityMenu";
 import { ANALYTICS_CONTEXT } from "metabase/collections/constants";
 
 const propTypes = {
-  collection: PropTypes.func,
+  collection: PropTypes.object,
   list: PropTypes.arrayOf(PropTypes.object),
 };
 
@@ -17,19 +17,19 @@ function NewCollectionItemMenu({ collection }) {
     {
       icon: "insight",
       title: t`Question`,
-      link: Urls.newQuestion({ mode: "notebook", collectionId: collection.id }),
+      link: newQuestion({ mode: "notebook", collectionId: collection.id }),
       event: `${ANALYTICS_CONTEXT};New Item Menu;Question Click`,
     },
     {
       icon: "dashboard",
       title: t`Dashboard`,
-      link: Urls.newDashboard(collection.id),
+      link: newDashboard(collection.id),
       event: `${ANALYTICS_CONTEXT};New Item Menu;Dashboard Click`,
     },
     {
       icon: "folder",
       title: t`Collection`,
-      link: Urls.newCollection(collection.id),
+      link: newCollection(collection.id),
       event: `${ANALYTICS_CONTEXT};New Item Menu;Collection Click`,
     },
   ];

frontend/src/metabase/collections/containers/CollectionSidebar/index.ts

@@ -0,0 +1 @@
+export { default } from "./CollectionSidebar";

frontend/src/metabase/components/Badge.jsx

@@ -45,7 +45,7 @@ function Badge({
       activeColor={activeColor}
       {...props}
     >
-      {icon && <BadgeIcon {...getIconProps(icon)} hasMargin={!!children} />}
+      {icon && <BadgeIcon {...getIconProps(icon)} $hasMargin={!!children} />}
       {children && <span className="text-wrap">{children}</span>}
     </MaybeLink>
   );

frontend/src/metabase/components/Badge.styled.jsx

@@ -5,6 +5,7 @@ import { css } from "@emotion/react";
 import { color } from "metabase/lib/colors";
 import Icon from "metabase/components/Icon";
 import Link from "metabase/core/components/Link";
+import { shouldForwardNonTransientProp } from "metabase/lib/styling/emotion";
 
 const propTypes = {
   to: PropTypes.oneOfType([PropTypes.string, PropTypes.bool]),
@@ -33,6 +34,8 @@ export const MaybeLink = styled(RawMaybeLink)`
   }
 `;
 
-export const BadgeIcon = styled(Icon)`
-  margin-right: ${props => (props.hasMargin ? "5px" : 0)};
+export const BadgeIcon = styled(Icon, {
+  shouldForwardProp: shouldForwardNonTransientProp,
+})`
+  margin-right: ${props => (props.$hasMargin ? "5px" : 0)};
 `;

frontend/src/metabase/components/ButtonBar.jsx

@@ -1,5 +1,5 @@
 /* eslint-disable react/prop-types */
-import React from "react";
+import React, { Children } from "react";
 import {
   ButtonBarCenter,
   ButtonBarLeft,
@@ -12,6 +12,8 @@ function normalizeArray(array) {
     array = array.filter(a => a);
     if (array.length === 0) {
       array = null;
+    } else {
+      array = Children.toArray(array);
     }
   }
   return array;

frontend/src/metabase/components/CollectionLanding/CollectionLanding.jsx

@@ -1,11 +1,11 @@
 /* eslint-disable react/prop-types */
 import React, { useState } from "react";
 
-import * as Urls from "metabase/lib/urls";
+import { extractCollectionId } from "metabase/lib/urls";
 
 import { PageWrapper } from "metabase/collections/components/Layout";
 import CollectionContent from "metabase/collections/containers/CollectionContent";
-import CollectionSidebar from "metabase/collections/containers/CollectionSidebar/CollectionSidebar";
+import CollectionSidebar from "metabase/collections/containers/CollectionSidebar";
 import { ContentBox } from "./CollectionLanding.styled";
 
 const CollectionLanding = ({ params: { slug }, children }) => {
@@ -16,7 +16,7 @@ const CollectionLanding = ({ params: { slug }, children }) => {
   const handleToggleMobileSidebar = () =>
     setShouldDisplayMobileSidebar(!shouldDisplayMobileSidebar);
 
-  const collectionId = Urls.extractCollectionId(slug);
+  const collectionId = extractCollectionId(slug);
   const isRoot = collectionId === "root";
 
   return (

frontend/src/metabase/components/Icon.tsx

@@ -7,6 +7,7 @@ import Tooltip from "metabase/components/Tooltip";
 import { loadIcon } from "metabase/icon_paths";
 import { color as c } from "metabase/lib/colors";
 import { stripLayoutProps } from "metabase/lib/utils";
+import { shouldForwardNonTransientProp } from "metabase/lib/styling/emotion";
 
 const MISSING_ICON_NAME = "unknown";
 
@@ -28,7 +29,7 @@ export const IconWrapper = styled.div<IconWrapperProps>`
   // special cases for certain icons
   // Icon-share has a taller viewbox than most so to optically center
   // the icon we need to translate it upwards
-  "> .icon.icon-share": {
+  & > .icon.icon-share {
     transform: translateY(-2px);
   }
   ${hover};
@@ -115,17 +116,17 @@ class BaseIcon extends Component<IconProps> {
       );
     } else if (icon.svg) {
       return (
-        <svg
+        <StyledSVG
           {...svgProps}
           dangerouslySetInnerHTML={{ __html: icon.svg }}
           ref={forwardedRef}
         />
       );
     } else if (icon.path) {
       return (
-        <svg {...svgProps} ref={forwardedRef}>
+        <StyledSVG {...svgProps} ref={forwardedRef}>
           <path d={icon.path} />
-        </svg>
+        </StyledSVG>
       );
     } else {
       console.warn(`Icon "${name}" must have an img, svg, or path`);
@@ -134,6 +135,10 @@ class BaseIcon extends Component<IconProps> {
   }
 }
 
+const StyledSVG = styled("svg", {
+  shouldForwardProp: shouldForwardNonTransientProp,
+})``;
+
 const BaseIconWithRef = forwardRef<HTMLElement, IconProps>(
   function BaseIconWithRef(props, ref) {
     return <BaseIcon {...props} forwardedRef={ref} />;

frontend/src/metabase/components/MetadataInfo/TableInfoPopover/TableInfoPopover.tsx

@@ -1,5 +1,4 @@
-import React from "react";
-import PropTypes from "prop-types";
+import React, { ReactElement } from "react";
 import { hideAll } from "tippy.js";
 
 import TippyPopover, {
@@ -8,6 +7,7 @@ import TippyPopover, {
 import { isVirtualCardId } from "metabase/lib/saved-questions/saved-questions";
 
 import { WidthBoundTableInfo } from "./TableInfoPopover.styled";
+import PropTypes from "prop-types";
 
 export const POPOVER_DELAY: [number, number] = [500, 300];
 
@@ -18,18 +18,20 @@ interface TableSubset {
 
 const propTypes = {
   table: PropTypes.shape({
-    id: PropTypes.oneOf([PropTypes.number, PropTypes.string]),
+    id: PropTypes.oneOfType([PropTypes.number, PropTypes.string]),
     description: PropTypes.string,
   }).isRequired,
   children: PropTypes.node,
   placement: PropTypes.string,
   offset: PropTypes.arrayOf(PropTypes.number),
 };
 
-type Props = { table: TableSubset } & Pick<
-  ITippyPopoverProps,
-  "children" | "placement" | "offset" | "delay"
->;
+type Props = {
+  table: TableSubset;
+  children: ReactElement;
+  placement: string;
+  offset: number[];
+} & Pick<ITippyPopoverProps, "children" | "placement" | "offset" | "delay">;
 
 const className = "table-info-popover";
 

frontend/src/metabase/components/TokenField.jsx

@@ -495,6 +495,7 @@ export default class TokenField extends Component {
       placeholder = null;
     }
 
+    const isControlledInput = !!this.onInputChange;
     const valuesList = (
       <ul
         className={cx(
@@ -534,9 +535,10 @@ export default class TokenField extends Component {
             // set size to be small enough that it fits in a parameter.
             size={10}
             placeholder={placeholder}
-            value={inputValue}
+            value={isControlledInput ? inputValue : undefined}
+            defaultValue={isControlledInput ? undefined : inputValue}
             onKeyDown={this.onInputKeyDown}
-            onChange={this.onInputChange}
+            onChange={isControlledInput ? this.onInputChange : undefined}
             onFocus={this.onInputFocus}
             onBlur={this.onInputBlur}
             onPaste={this.onInputPaste}

frontend/src/metabase/core/components/CheckBox/CheckBox.tsx

@@ -54,15 +54,17 @@ const CheckBox = forwardRef(function Checkbox(
   }: CheckBoxProps,
   ref: Ref<HTMLLabelElement>,
 ): JSX.Element {
+  const isControlledCheckBoxInput = !!onChange;
   return (
     <CheckBoxRoot ref={ref} {...props}>
       <CheckBoxInput
         type="checkbox"
-        checked={checked}
+        checked={isControlledCheckBoxInput ? !!checked : undefined}
+        defaultChecked={isControlledCheckBoxInput ? undefined : !!checked}
         size={size}
         disabled={disabled}
         autoFocus={autoFocus}
-        onChange={onChange}
+        onChange={isControlledCheckBoxInput ? onChange : undefined}
         onFocus={onFocus}
         onBlur={onBlur}
       />

frontend/src/metabase/core/components/Link/Link.styled.tsx

@@ -2,8 +2,11 @@ import styled from "@emotion/styled";
 import { display, space, hover, color } from "styled-system";
 import { Link } from "react-router";
 import { color as colors } from "metabase/lib/colors";
+import { shouldForwardNonTransientProp } from "metabase/lib/styling/emotion";
 
-export const LinkRoot = styled(Link)`
+export const LinkRoot = styled(Link, {
+  shouldForwardProp: shouldForwardNonTransientProp,
+})`
   ${display};
   ${space};
   ${hover};

frontend/src/metabase/lib/styling/emotion.ts

@@ -0,0 +1,5 @@
+import isPropValid from "@emotion/is-prop-valid";
+
+export function shouldForwardNonTransientProp(propName: string): boolean {
+  return !propName.startsWith("$") && isPropValid(propName);
+}

frontend/src/metabase/parameters/components/ParameterValueWidget.jsx

@@ -181,7 +181,7 @@ export default class ParameterValueWidget extends Component {
               isFullscreen={isFullscreen}
               hasValue={hasValue}
               noReset={noReset}
-              noPopover={noPopover}
+              noPopover={!!noPopover}
               isFocused={isFocused}
               setValue={setValue}
             />
@@ -216,7 +216,7 @@ export default class ParameterValueWidget extends Component {
                   isFullscreen={isFullscreen}
                   hasValue={hasValue}
                   noReset={noReset}
-                  noPopover={noPopover}
+                  noPopover={!!noPopover}
                   isFocused={isFocused}
                   setValue={setValue}
                 />

frontend/src/metabase/query_builder/components/view/sidebars/SummarizeSidebar/AddAggregationButton/AddAggregationButton.jsx

@@ -17,7 +17,7 @@ const propTypes = {
 
 const LABEL = t`Add a metric`;
 
-export const AddAggregationButton = ({ query, shouldShowLabel }) => {
+export const AddAggregationButton = ({ query, shouldShowLabel = false }) => {
   return (
     <PopoverWithTrigger
       triggerElement={

frontend/src/metabase/questions/components/CollectionBadge.jsx

@@ -8,7 +8,7 @@ import { PLUGIN_COLLECTIONS } from "metabase/plugins";
 
 const propTypes = {
   collection: PropTypes.object,
-  analyticsContext: PropTypes.string.isRequired,
+  analyticsContext: PropTypes.string,
   className: PropTypes.string,
 };
 
@@ -25,6 +25,7 @@ function CollectionBadge({ collection, analyticsContext, className }) {
   if (!collection) {
     return null;
   }
+
   const isRegular = PLUGIN_COLLECTIONS.isRegularCollection(collection);
   const icon = {
     ...collection.getIcon(),