From 35755d316c0faa746900d14cd304834a7f7863bc Mon Sep 17 00:00:00 2001 From: Phillip Lovelace Date: Wed, 4 Feb 2026 17:56:03 -0800 Subject: [PATCH 1/7] feat(pds-dropdown-menu): allow raw anchor and button elements --- .../pds-dropdown-menu/pds-dropdown-menu.tsx | 109 +++++++++++++----- 1 file changed, 80 insertions(+), 29 deletions(-) diff --git a/libs/core/src/components/pds-dropdown-menu/pds-dropdown-menu.tsx b/libs/core/src/components/pds-dropdown-menu/pds-dropdown-menu.tsx index eac9bbc79..3083401e7 100644 --- a/libs/core/src/components/pds-dropdown-menu/pds-dropdown-menu.tsx +++ b/libs/core/src/components/pds-dropdown-menu/pds-dropdown-menu.tsx @@ -8,6 +8,11 @@ import { computePosition, autoUpdate, } from '@floating-ui/dom'; +/** + * Union type for focusable menu items (component or raw elements) + */ +type MenuItemElement = HTMLPdsDropdownMenuItemElement | HTMLAnchorElement | HTMLButtonElement; + /** * @part menu-panel - Exposes the dropdown menu container for styling. */ @@ -21,7 +26,7 @@ export class PdsDropdownMenu implements BasePdsProps { private triggerEl: HTMLElement; private panelEl: HTMLPdsBoxElement; private isOpen: boolean = false; - private menuItems: HTMLPdsDropdownMenuItemElement[] = []; + private menuItems: MenuItemElement[] = []; private cleanupAutoUpdate: (() => void) | null = null; @Element() host: HTMLPdsDropdownMenuElement; @@ -70,17 +75,28 @@ export class PdsDropdownMenu implements BasePdsProps { // Get all elements assigned to this slot const assignedElements = this.slotEl.assignedElements(); - // ensure assignedElements only contains pds-dropdown-menu-item or pds-dropdown-menu-separator - // if there are other elements, throw an error - const invalidElements = assignedElements.filter(el => el.tagName.toLowerCase() !== 'pds-dropdown-menu-item' && el.tagName.toLowerCase() !== 'pds-dropdown-menu-separator'); + // Allowed elements: pds-dropdown-menu-item, pds-dropdown-menu-separator, , + Item 1 + Delete + + `, + }); + + const component = page.rootInstance; + const contentSlot = page.root.shadowRoot?.querySelector('pds-box > slot'); + + // Create a mock event with menu item and raw anchor + const mockEvent = { + target: contentSlot, + assignedElements: () => [ + page.body.querySelector('pds-dropdown-menu-item'), + page.body.querySelector('a') + ] + }; + + // Spy on console.warn to ensure no warning is logged + const warnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}); + + // Call should not warn + component.handleSlotChange(mockEvent as any); + + // No warning should be logged for allowed elements + expect(warnSpy).not.toHaveBeenCalled(); + + // Both elements should be in menuItems for keyboard navigation + expect(component.menuItems.length).toBe(2); + + warnSpy.mockRestore(); + }); + + it('allows raw + Item 1 + + + `, + }); + + const component = page.rootInstance; + const contentSlot = page.root.shadowRoot?.querySelector('pds-box > slot'); + + // Get the raw button (not the trigger) + const rawButton = page.body.querySelectorAll('button')[1]; + + // Create a mock event with menu item and raw button + const mockEvent = { + target: contentSlot, + assignedElements: () => [ + page.body.querySelector('pds-dropdown-menu-item'), + rawButton + ] + }; + + // Spy on console.warn to ensure no warning is logged + const warnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}); + + // Call should not warn + component.handleSlotChange(mockEvent as any); + + // No warning should be logged for allowed elements + expect(warnSpy).not.toHaveBeenCalled(); + + // Both elements should be in menuItems for keyboard navigation + expect(component.menuItems.length).toBe(2); + + warnSpy.mockRestore(); + }); + + it('skips disabled items during keyboard navigation', async () => { + const page = await newSpecPage({ + components: [PdsDropdownMenu, PdsDropdownMenuItem, PdsBox], + html: ` + + + Item 1 + Item 2 (Disabled) + Item 3 + + `, + }); + + const component = page.rootInstance; + component.isOpen = true; + + // Setup slot change to populate menuItems + const contentSlot = page.root?.shadowRoot?.querySelector('pds-box > slot'); + const mockEvent = { + target: contentSlot, + assignedElements: () => Array.from(page.body.querySelectorAll('pds-dropdown-menu-item')) + }; + component.handleSlotChange(mockEvent as any); + + // Verify we have 3 items + expect(component.menuItems.length).toBe(3); + + // Start at first item + component.currentFocusIndex = 0; + + // Call focusNextItem - should skip disabled item 2 and go to item 3 + component.focusNextItem(); + expect(component.currentFocusIndex).toBe(2); // Should skip index 1 (disabled) }); it('applies specified placement', async () => { @@ -368,11 +494,11 @@ describe('pds-dropdown-menu', () => { const mockTrigger = document.createElement('button'); component.triggerEl = mockTrigger; - // Create menu items for testing with first item available + // Create menu items for testing with proper tagName property component.menuItems = [ - { disabled: false, focus: jest.fn() } as any, - { disabled: false, focus: jest.fn() } as any, - { disabled: true, focus: jest.fn() } as any + { tagName: 'PDS-DROPDOWN-MENU-ITEM', disabled: false, focus: jest.fn() } as any, + { tagName: 'PDS-DROPDOWN-MENU-ITEM', disabled: false, focus: jest.fn() } as any, + { tagName: 'PDS-DROPDOWN-MENU-ITEM', disabled: true, focus: jest.fn() } as any ]; // Mock document.activeElement to be the trigger From 6cc376d0d766fdcdadfc03d09cba60801f271599 Mon Sep 17 00:00:00 2001 From: Phillip Lovelace Date: Wed, 4 Feb 2026 17:57:41 -0800 Subject: [PATCH 4/7] docs(pds-dropdown-menu): document raw element support and styling --- .../docs/pds-dropdown-menu.mdx | 112 ++++++++++++++++++ 1 file changed, 112 insertions(+) diff --git a/libs/core/src/components/pds-dropdown-menu/docs/pds-dropdown-menu.mdx b/libs/core/src/components/pds-dropdown-menu/docs/pds-dropdown-menu.mdx index 5dce2de1b..60981793f 100644 --- a/libs/core/src/components/pds-dropdown-menu/docs/pds-dropdown-menu.mdx +++ b/libs/core/src/components/pds-dropdown-menu/docs/pds-dropdown-menu.mdx @@ -284,6 +284,118 @@ Menu items can open links in a new tab using the `external` boolean prop. This d >**Note:** You can also use the `target` prop for more control (e.g., `target="_blank"`, `target="_parent"`). The `target` prop takes precedence over `external` if both are set. +### Mixing Menu Items with Raw Elements + +In most cases, use `pds-dropdown-menu-item` for menu options. However, for edge cases requiring native browser or framework behavior (such as handling non-GET HTTP methods or framework-specific data attributes), you can mix in raw `` or ` + Delete + +`, + webComponent:` + + + + + Actions + View + + Edit + Archive + + + Delete + +` + }} +> + +
+ + Actions + View + + Edit + Archive + + + Delete + +
+ + +**Raw element attributes:** +- Add `class="destructive"` for danger/destructive text color +- Add `aria-disabled="true"` to visually disable an anchor element +- Add `disabled` attribute to visually disable a button element + +>**Note:** Use `pds-dropdown-menu-item` for most cases. Raw elements should only be used when you need native browser behavior that cannot work through the component's Shadow DOM (e.g., framework-specific data attributes for form submissions). Due to Shadow DOM limitations, hover and focus states for raw elements must be styled by the consuming application's CSS. + ### With Disabled Items Menu items can be disabled using the disabled attribute. From c925b8869f709b6b913d1a1b1df3b084668d1fbe Mon Sep 17 00:00:00 2001 From: Phillip Lovelace Date: Thu, 5 Feb 2026 08:41:48 -0800 Subject: [PATCH 5/7] fix(pds-dropdown-menu): ensure slotted element styles override app defaults --- .../src/components/pds-dropdown-menu/pds-dropdown-menu.scss | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/libs/core/src/components/pds-dropdown-menu/pds-dropdown-menu.scss b/libs/core/src/components/pds-dropdown-menu/pds-dropdown-menu.scss index cb6e5134d..4322d4b2d 100644 --- a/libs/core/src/components/pds-dropdown-menu/pds-dropdown-menu.scss +++ b/libs/core/src/components/pds-dropdown-menu/pds-dropdown-menu.scss @@ -35,7 +35,7 @@ border: 0; border-radius: var(--pine-dimension-xs); box-sizing: border-box; - color: var(--pine-color-text); + color: var(--pine-color-text) !important; cursor: pointer; display: flex; flex-grow: 1; @@ -44,13 +44,13 @@ margin: calc(var(--pine-border-width) + 2px); padding: var(--pine-dimension-xs); text-align: start; - text-decoration: none; + text-decoration: none !important; width: calc(100% - calc(var(--pine-border-width) + 2px) * 2); } // Destructive variant for raw elements ::slotted(.destructive) { - color: var(--pine-color-danger); + color: var(--pine-color-danger) !important; } // Disabled state for raw elements (using attribute selectors, not pseudo-classes) From f674159069b83387c57b0d9aad487c9c0099f201 Mon Sep 17 00:00:00 2001 From: Phillip Lovelace Date: Thu, 5 Feb 2026 08:51:23 -0800 Subject: [PATCH 6/7] fix(pds-table): defer default sort to allow child components to initialize --- libs/core/src/components/pds-table/pds-table.tsx | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/libs/core/src/components/pds-table/pds-table.tsx b/libs/core/src/components/pds-table/pds-table.tsx index 79d368aee..8b117bd92 100644 --- a/libs/core/src/components/pds-table/pds-table.tsx +++ b/libs/core/src/components/pds-table/pds-table.tsx @@ -86,9 +86,12 @@ export class PdsTable { } // Apply default sort if specified + // Use requestAnimationFrame to defer until child components are fully initialized if (this.defaultSortColumn) { - void this.applyDefaultSort().catch((err) => { - console.warn('Failed to apply default sort.', err); + requestAnimationFrame(() => { + void this.applyDefaultSort().catch((err) => { + console.warn('Failed to apply default sort.', err); + }); }); } } From 1f6f3618f01b758c24a378c07f497293fa400b45 Mon Sep 17 00:00:00 2001 From: Phillip Lovelace Date: Thu, 5 Feb 2026 09:36:48 -0800 Subject: [PATCH 7/7] test(pds-table): add flushAnimationFrame helper to improve test reliability --- .../core/src/components/pds-table/test/pds-table.spec.tsx | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/libs/core/src/components/pds-table/test/pds-table.spec.tsx b/libs/core/src/components/pds-table/test/pds-table.spec.tsx index d6a3e5818..af39f72d3 100644 --- a/libs/core/src/components/pds-table/test/pds-table.spec.tsx +++ b/libs/core/src/components/pds-table/test/pds-table.spec.tsx @@ -6,6 +6,9 @@ import { PdsTableBody } from '../pds-table-body/pds-table-body'; import { PdsTableRow } from '../pds-table-row/pds-table-row'; import { PdsTableCell } from '../pds-table-cell/pds-table-cell'; +// Helper to wait for requestAnimationFrame to complete +const flushAnimationFrame = () => new Promise((resolve) => requestAnimationFrame(resolve)); + describe('pds-table', () => { it('renders', async () => { const page = await newSpecPage({ @@ -399,6 +402,7 @@ describe('pds-table', () => { `, }); + await flushAnimationFrame(); await page.waitForChanges(); const tableBody = page.body.querySelector('pds-table-body') as HTMLElement; @@ -436,6 +440,7 @@ describe('pds-table', () => { `, }); + await flushAnimationFrame(); await page.waitForChanges(); const tableBody = page.body.querySelector('pds-table-body') as HTMLElement; @@ -465,6 +470,7 @@ describe('pds-table', () => { `, }); + await flushAnimationFrame(); await page.waitForChanges(); const headCells = page.body.querySelectorAll('pds-table-head-cell'); @@ -496,6 +502,7 @@ describe('pds-table', () => { `, }); + await flushAnimationFrame(); await page.waitForChanges(); expect(consoleWarnSpy).toHaveBeenCalledWith('Default sort column "NonExistent" not found.'); @@ -547,6 +554,7 @@ describe('pds-table', () => { `, }); + await flushAnimationFrame(); await page.waitForChanges(); // Should not throw and header should still be marked active