Update MenuItem to use AppLink

packages/components/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@labkey/components",
-  "version": "6.48.0",
+  "version": "6.48.1",
   "description": "Components, models, actions, and utility functions for LabKey applications and pages",
   "sideEffects": false,
   "files": [

packages/components/releaseNotes/components.md

@@ -1,6 +1,12 @@
 # @labkey/components
 Components, models, actions, and utility functions for LabKey applications and pages
 
+### version 6.48.1
+*Released*: 10 June 2025
+- Update `AppLink` to extend all props of `AnchorHTMLAttributes`
+- Update `MenuItem` to use `AppLink`
+- Update all usages of `MenuItem` to pass `AppURL` where possible
+
 ### version 6.48.0
 *Released*: 9 June 2025
 - Issue 52556: Add data-fieldkey attribute to grid header elements and input elements (part 2)

packages/components/src/internal/components/domainproperties/assay/models.ts

@@ -21,6 +21,7 @@ import { DomainDesign, FieldErrors } from '../models';
 import { AppURL } from '../../../url/AppURL';
 import { getAppHomeFolderPath } from '../../../app/utils';
 import { Container } from '../../base/models/Container';
+import { ASSAYS_KEY } from '../../../app/constants';
 
 // See ExpProtocol.Status in 'platform' repository.
 export enum Status {
@@ -242,6 +243,6 @@ export class AssayProtocolModel extends ImmutableRecord({
     }
 
     getUrl(): AppURL {
-        return AppURL.create('assays', this.providerName, this.name);
+        return AppURL.create(ASSAYS_KEY, this.providerName, this.name);
     }
 }

packages/components/src/internal/components/entities/AssayResultsForSamplesButton.test.tsx

@@ -23,7 +23,7 @@ describe('AssayResultsForSamplesButton', () => {
         // expect(document.querySelectorAll('.lk-menu-item').prop('nounPlural')).toBe('samples');
         expect(document.querySelector('.lk-menu-item a')).toHaveAttribute(
             'href',
-            '#/assays/sampleresults?selectionKey=model'
+            '/assays/sampleresults?selectionKey=model'
         );
     });
 
@@ -32,7 +32,7 @@ describe('AssayResultsForSamplesButton', () => {
         expect(document.querySelector('.lk-menu-item a')).toBeInTheDocument();
         expect(document.querySelector('.lk-menu-item a')).toHaveAttribute(
             'href',
-            '#/assays/sampleresults?selectionKey=model&picklistName=query'
+            '/assays/sampleresults?selectionKey=model&picklistName=query'
         );
     });
 

packages/components/src/internal/components/entities/AssayResultsForSamplesButton.tsx

@@ -9,19 +9,19 @@ import { User } from '../base/models/User';
 import { incrementClientSideMetricCount } from '../../actions';
 import { userCanReadAssays } from '../../app/utils';
 import { ResponsiveMenuButton } from '../buttons/ResponsiveMenuButton';
-import { SampleTypeDataType } from './constants';
+
 import { MAX_SELECTION_ACTION_ROWS } from '../../constants';
 
+import { SampleTypeDataType } from './constants';
+
 function getAssayResultsHref(
     model: QueryModel,
     picklistName?: string,
     isAssay?: boolean,
-    sampleFieldKey?: string,
-): string {
-    // TODO: update this to return AppURL when MenuItem is updated to render AppLink and take AppURl
+    sampleFieldKey?: string
+): AppURL {
     const params = getURLParamsForSampleSelectionKey(model, picklistName, isAssay, sampleFieldKey);
-    const actionUrl = AppURL.create(ASSAYS_KEY, 'sampleresults').addParams(params);
-    return actionUrl.toHref();
+    return AppURL.create(ASSAYS_KEY, 'sampleresults').addParams(params);
 }
 
 interface Props {
@@ -65,7 +65,7 @@ export const AssayResultsForSamplesButton: FC<Props> = memo(props => {
     if (!userCanReadAssays(user)) return null;
 
     return (
-        <ResponsiveMenuButton className="sample-reports-menu" text="Reports" asSubMenu={asSubMenu} >
+        <ResponsiveMenuButton className="sample-reports-menu" text="Reports" asSubMenu={asSubMenu}>
             <AssayResultsForSamplesMenuItem {...props} />
         </ResponsiveMenuButton>
     );

packages/components/src/internal/components/menus/SelectionMenuItem.tsx

@@ -21,9 +21,10 @@ import { QueryModel } from '../../../public/QueryModel/QueryModel';
 import { MenuItem } from '../../dropdowns';
 import { useOverlayTriggerState } from '../../OverlayTrigger';
 import { Popover } from '../../Popover';
+import { AppURL } from '../../url/AppURL';
 
 interface Props {
-    href?: string;
+    href?: string | AppURL;
     maxSelection?: number;
     maxSelectionDisabledMsg?: string;
     nounPlural: string; // always used, doesn't need default value

packages/components/src/internal/components/productnavigation/ProductNavigationItem.tsx

@@ -1,4 +1,5 @@
 import React, { FC, memo, PropsWithChildren, useCallback, useState } from 'react';
+
 import { AppLink } from '../../url/AppLink';
 import { AppURL } from '../../url/AppURL';
 
@@ -13,6 +14,9 @@ export const ProductNavigationItem: FC<ProductClickableItemProps> = memo(({ chil
     const onEnter = useCallback(() => setHovered(true), [setHovered]);
     const onLeave = useCallback(() => setHovered(false), [setHovered]);
 
+    // FIXME Do not use onMouseEnter or onMouseLeave.
+    // They are only needed because this applies a new CSS class on hover.
+    // Update to use a css :hover selector to apply the styling.
     return (
         <AppLink
             to={url}

packages/components/src/internal/components/samples/DisableableMenuItem.tsx

@@ -6,12 +6,13 @@ import { useOverlayTriggerState } from '../../OverlayTrigger';
 import { MenuItem } from '../../dropdowns';
 import { Popover } from '../../Popover';
 import { Placement } from '../../useOverlayPositioning';
+import { AppURL } from '../../url/AppURL';
 
 export interface DisableableMenuItemProps extends PropsWithChildren {
     className?: string;
     disabled?: boolean;
     disabledMessage?: ReactNode;
-    href?: string;
+    href?: string | AppURL;
     onClick?: () => void;
     placement?: Placement;
 }

packages/components/src/internal/components/search/FindAndSearchDropdown.tsx

@@ -34,13 +34,10 @@ export const FindAndSearchDropdown: FC<Props> = memo(props => {
     const [findField, setFindField] = useState<FindField>(undefined);
     const [showFindModal, setShowFindModal] = useState<boolean>(false);
 
-    const onShowFind = useCallback(
-        (findField: FindField) => {
-            setFindField(findField);
-            setShowFindModal(true);
-        },
-        [setShowFindModal]
-    );
+    const onShowFind = useCallback((findField_: FindField) => {
+        setFindField(findField_);
+        setShowFindModal(true);
+    }, []);
 
     const onHideFindModal = useCallback(() => {
         setFindField(undefined);
@@ -60,8 +57,8 @@ export const FindAndSearchDropdown: FC<Props> = memo(props => {
     }, [api]);
 
     const capNoun = capitalizeFirstChar(findNounPlural);
-    const findByBarcodeClicked = useCallback(() => onShowFind(UNIQUE_ID_FIND_FIELD), []);
-    const findByIdClicked = useCallback(() => onShowFind(SAMPLE_ID_FIND_FIELD), []);
+    const findByBarcodeClicked = useCallback(() => onShowFind(UNIQUE_ID_FIND_FIELD), [onShowFind]);
+    const findByIdClicked = useCallback(() => onShowFind(SAMPLE_ID_FIND_FIELD), [onShowFind]);
 
     return (
         <>
@@ -76,7 +73,7 @@ export const FindAndSearchDropdown: FC<Props> = memo(props => {
                         </MenuItem>
                     </>
                 )}
-                <MenuItem onClick={onSampleFinder} href={FIND_SAMPLES_BY_FILTER_HREF.toHref()}>
+                <MenuItem onClick={onSampleFinder} href={FIND_SAMPLES_BY_FILTER_HREF}>
                     <i className="fa fa-sitemap" /> Sample Finder
                 </MenuItem>
                 {!!onSearch && (
@@ -96,3 +93,4 @@ export const FindAndSearchDropdown: FC<Props> = memo(props => {
         </>
     );
 });
+FindAndSearchDropdown.displayName = 'FindAndSearchDropdown';

packages/components/src/internal/dropdowns.tsx

@@ -18,6 +18,8 @@ import classNames from 'classnames';
 
 import { generateId } from './util/utils';
 import { cancelEvent } from './events';
+import { AppLink } from './url/AppLink';
+import { AppURL } from './url/AppURL';
 
 export type BSStyle = 'success' | 'danger' | 'default' | 'primary' | 'info';
 const DROPDOWN_MENU_CLASS = 'dropdown-menu';
@@ -300,7 +302,7 @@ export interface MenuItemProps {
     children: ReactNode;
     className?: string;
     disabled?: boolean;
-    href?: string;
+    href?: string | AppURL;
     onClick?: () => void;
     onMouseEnter?: () => void;
     onMouseLeave?: () => void;
@@ -342,12 +344,12 @@ export const MenuItem = forwardRef<HTMLLIElement, MenuItemProps>((props, ref) =>
         },
         [disabled, href, onClick]
     );
-    // TODO: Use AppLink (needs support for role and rel), this will be completed in a follow-on PR
+
     return (
         <li className={className} role="presentation" ref={ref} onMouseEnter={onMouseEnter} onMouseLeave={onMouseLeave}>
-            <a onClick={onClick_} href={href} rel={rel} role="menuitem" target={target} title={title}>
+            <AppLink onClick={onClick_} rel={rel} role="menuitem" target={target} title={title} to={href}>
                 {children}
-            </a>
+            </AppLink>
         </li>
     );
 });

packages/components/src/internal/renderers/DefaultRenderer.tsx

@@ -16,8 +16,6 @@
 import React, { FC, memo } from 'react';
 import { List } from 'immutable';
 
-import { Link } from 'react-router-dom';
-
 import { QueryColumn } from '../../public/QueryColumn';
 
 import { getDataStyling } from '../util/utils';

packages/components/src/internal/url/AppLink.tsx

@@ -1,4 +1,4 @@
-import React, { FC, memo, PropsWithChildren, StyleHTMLAttributes, MouseEvent } from 'react';
+import React, { AnchorHTMLAttributes, DetailedHTMLProps, FC, memo, useMemo } from 'react';
 
 import { Link } from 'react-router-dom';
 
@@ -12,7 +12,7 @@ const TARGET_BLANK = '_blank';
 const URL_REL = 'noopener noreferrer';
 
 /**
- * If the given href points to our current container & product, return the react router path, in all other cases
+ * If the given href points to our current container & product, return the react-router path, in all other cases
  * return undefined.
  * @param href
  */
@@ -33,63 +33,51 @@ export function parseAppPath(href: string): string | undefined {
     return undefined;
 }
 
-/**
- * DO NOT USE onMouseEnter or onMouseLeave, they are only needed because  the ProductNavigationItem applies a new CSS
- * class on hover. This component will be updated in the near future to use a css :hover selector to apply the styling.
- */
-interface Props extends PropsWithChildren {
-    className?: string;
-    onClick?: (event: MouseEvent<HTMLAnchorElement>) => void;
-    onMouseEnter?: (event: MouseEvent<HTMLAnchorElement>) => void;
-    onMouseLeave?: (event: MouseEvent<HTMLAnchorElement>) => void;
-    style?: StyleHTMLAttributes<HTMLAnchorElement>;
+/** This is a subset of AnchorHTMLAttributes<HTMLAnchorElement> that are passed through to the anchor tag. */
+type InheritedHTMLAnchorProps = Omit<
+    DetailedHTMLProps<AnchorHTMLAttributes<HTMLAnchorElement>, HTMLAnchorElement>,
+    'href' // overridden by AppLink which uses "to" instead
+>;
+
+/** These props are specific to <AppLink>. */
+interface Props extends InheritedHTMLAnchorProps {
     targetBlank?: boolean;
-    title?: string;
-    to: string | AppURL;
+    to: string | AppURL | undefined;
 }
 
 /**
  * Use this when you need to generate an anchor tag. This should be preferred to all usages of <a> or <Link>, as it
  * handles all the corner cases around moving within our apps, between our apps, to other parts of LKS, and externally.
  */
 export const AppLink: FC<Props> = memo(props => {
-    const { children, className, onClick, onMouseEnter, onMouseLeave, style, targetBlank, title, to } = props;
-    let appPath;
+    const { children, rel, target, targetBlank, to, ...anchorProps } = props;
 
-    if (to instanceof AppURL && to.isAppPath()) {
-        appPath = to.toString();
-    } else if (typeof to === 'string') {
-        appPath = parseAppPath(to);
-    }
+    const appPath = useMemo<string | undefined>(() => {
+        if (to instanceof AppURL && to.isAppPath()) {
+            return to.toString();
+        } else if (typeof to === 'string') {
+            return parseAppPath(to);
+        }
+
+        return undefined;
+    }, [to]);
 
     // React Router <Link> components render as empty strings in the test environment, so we force them to render as
     // anchor tags in our tests.
     if (appPath && !isTestEnv()) {
         return (
-            <Link
-                className={className}
-                onClick={onClick}
-                onMouseEnter={onMouseEnter}
-                onMouseLeave={onMouseLeave}
-                style={style}
-                to={appPath}
-            >
+            <Link {...anchorProps} to={appPath}>
                 {children}
             </Link>
         );
     }
 
     return (
         <a
-            className={className}
-            href={to.toString()}
-            onClick={onClick}
-            onMouseEnter={onMouseEnter}
-            onMouseLeave={onMouseLeave}
-            rel={targetBlank ? URL_REL : undefined}
-            style={style}
-            target={targetBlank ? TARGET_BLANK : undefined}
-            title={title}
+            {...anchorProps}
+            href={to?.toString()}
+            rel={targetBlank ? URL_REL : rel}
+            target={targetBlank ? TARGET_BLANK : target}
         >
             {children}
         </a>