From 9d7d32ce28e526c985bdd0be6875439332b3c6a6 Mon Sep 17 00:00:00 2001 From: Daniel Spofford <868014+danielspofford@users.noreply.github.com> Date: Sun, 21 Sep 2025 23:05:34 -0600 Subject: [PATCH 1/2] fix anchor scrolling --- src/DeckardSchema.tsx | 152 +++++++----- src/__tests__/anchor-scrolling.test.tsx | 312 ++++++++++++++++++++++++ src/property/PropertyField.tsx | 16 +- src/property/PropertyRow.tsx | 16 +- 4 files changed, 426 insertions(+), 70 deletions(-) create mode 100644 src/__tests__/anchor-scrolling.test.tsx diff --git a/src/DeckardSchema.tsx b/src/DeckardSchema.tsx index 3edbfa1..49d5670 100644 --- a/src/DeckardSchema.tsx +++ b/src/DeckardSchema.tsx @@ -423,6 +423,7 @@ export const DeckardSchema: React.FC = ({ results: 0, }); const [focusedProperty, setFocusedProperty] = useState(null); + const [scrollTarget, setScrollTarget] = useState(null); const [keyboardModalOpen, setKeyboardModalOpen] = useState(false); const [examplesHidden, setExamplesHidden] = useState(false); @@ -496,47 +497,80 @@ export const DeckardSchema: React.FC = ({ setPropertyStates(newStates); }, [schema, autoExpand]); - // Handle URL hash navigation - only on mount - // Handle URL hash navigation - only on mount + // Handle URL hash navigation and hash changes useEffect(() => { - const hash = typeof window !== 'undefined' ? window.location.hash : ''; - if (hash) { - const fieldKey = hashToPropertyKey(hash); + let isInitialLoad = true; - // Update property states to expand path to target - setPropertyStates(prev => { - const newStates = { ...prev }; + const handleHashNavigation = () => { + const hash = typeof window !== 'undefined' ? window.location.hash : ''; + if (hash) { + const fieldKey = hashToPropertyKey(hash); - // Expand all parent paths to make the target field visible - const pathParts = fieldKey.split('.'); + // Update property states to expand path to target + setPropertyStates(prev => { + const newStates = { ...prev }; - for (let i = 1; i <= pathParts.length; i++) { - const parentPath = pathParts.slice(0, i).join('.'); + // Expand all parent paths to make the target field visible + const pathParts = fieldKey.split('.'); - if (newStates[parentPath]) { - newStates[parentPath] = { - ...newStates[parentPath], - expanded: true, - }; - } else { - newStates[parentPath] = { - expanded: true, - hasDetails: true, - matchesSearch: true, - isDirectMatch: false, - hasNestedMatches: false, - }; + for (let i = 1; i <= pathParts.length; i++) { + const parentPath = pathParts.slice(0, i).join('.'); + + if (newStates[parentPath]) { + newStates[parentPath] = { + ...newStates[parentPath], + expanded: true, + }; + } else { + newStates[parentPath] = { + expanded: true, + hasDetails: true, + matchesSearch: true, + isDirectMatch: false, + hasNestedMatches: false, + }; + } } - } - return newStates; - }); + return newStates; + }); + + // Set the focused property state for proper styling + setFocusedProperty(fieldKey); - // Set the focused property state for proper styling - setFocusedProperty(fieldKey); + // Set a flag to trigger scrolling after state updates + setScrollTarget(fieldKey); + } + }; + + // Handle initial hash on mount + handleHashNavigation(); + isInitialLoad = false; + + // Listen for hash changes + if (typeof window !== 'undefined') { + window.addEventListener('hashchange', handleHashNavigation); + return () => + window.removeEventListener('hashchange', handleHashNavigation); } }, []); + // Handle scrolling after state updates complete + useEffect(() => { + if (scrollTarget && typeof document !== 'undefined') { + const targetElement = document.getElementById( + propertyKeyToHash(scrollTarget) + ); + if (targetElement && typeof targetElement.scrollIntoView === 'function') { + targetElement.scrollIntoView({ + behavior: 'smooth', + block: 'start', + }); + } + setScrollTarget(null); + } + }, [scrollTarget, propertyStates]); + // Update search results useEffect(() => { setSearchState(prev => ({ @@ -744,37 +778,36 @@ export const DeckardSchema: React.FC = ({ // Navigation helpers const focusProperty = useCallback((propertyKey: string) => { setFocusedProperty(propertyKey); - setTimeout(() => { - if (typeof document === 'undefined' || typeof window === 'undefined') { - return; - } - const element = document.querySelector( - `[data-property-key="${propertyKey}"]` + if (typeof document === 'undefined' || typeof window === 'undefined') { + return; + } + + const element = document.querySelector( + `[data-property-key="${propertyKey}"]` + ) as HTMLElement; + if (element) { + // Focus the header container to trigger :focus-within like clicking does + const headerContainer = element.querySelector( + '.property-header-container' ) as HTMLElement; - if (element) { - // Focus the header container to trigger :focus-within like clicking does - const headerContainer = element.querySelector( - '.property-header-container' - ) as HTMLElement; - if (headerContainer) { - headerContainer.focus(); - } else { - element.setAttribute('tabindex', '-1'); - element.focus(); - } + if (headerContainer) { + headerContainer.focus(); + } else { + element.setAttribute('tabindex', '-1'); + element.focus(); + } - const rect = element.getBoundingClientRect(); - const viewportHeight = - typeof window !== 'undefined' ? window.innerHeight : 0; - const isInView = rect.top >= 0 && rect.bottom <= viewportHeight; + const rect = element.getBoundingClientRect(); + const viewportHeight = + typeof window !== 'undefined' ? window.innerHeight : 0; + const isInView = rect.top >= 0 && rect.bottom <= viewportHeight; - // Only scroll if the element is not fully visible - if (!isInView && typeof element.scrollIntoView === 'function') { - element.scrollIntoView({ behavior: 'smooth', block: 'center' }); - } + // Only scroll if the element is not fully visible + if (!isInView && typeof element.scrollIntoView === 'function') { + element.scrollIntoView({ behavior: 'smooth', block: 'start' }); } - }, 50); + } }, []); const getAllNavigableProperties = useCallback(() => { @@ -1134,10 +1167,13 @@ export const DeckardSchema: React.FC = ({ const anchor = `#${propertyKeyToHash(propertyKey)}`; const url = `${window.location.origin}${window.location.pathname}${anchor}`; - // Update the URL in the address bar (this will trigger native browser scrolling) + // Update the URL in the address bar window.location.hash = anchor; - // Focus the current element + // Set scroll target for deterministic scrolling after state updates + setScrollTarget(propertyKey); + + // Focus immediately since it doesn't depend on DOM updates element.focus(); try { diff --git a/src/__tests__/anchor-scrolling.test.tsx b/src/__tests__/anchor-scrolling.test.tsx new file mode 100644 index 0000000..93bc39d --- /dev/null +++ b/src/__tests__/anchor-scrolling.test.tsx @@ -0,0 +1,312 @@ +import { render, fireEvent, waitFor } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import { DeckardSchema } from '../DeckardSchema'; +import { JsonSchema } from '../types'; + +// Mock scrollIntoView for keyboard navigation testing +const mockScrollIntoView = vi.fn(); +Element.prototype.scrollIntoView = mockScrollIntoView; + +describe('Anchor Scrolling', () => { + const mockSchema: JsonSchema = { + type: 'object', + properties: { + basicProperty: { + type: 'string', + description: 'A basic string property', + }, + nestedObject: { + type: 'object', + description: 'An object with nested properties', + properties: { + nestedString: { + type: 'string', + description: 'A nested string property', + }, + }, + }, + oneOfProperty: { + description: 'A property with oneOf options', + oneOf: [ + { + type: 'string', + title: 'String Option', + description: 'String configuration', + }, + { + type: 'object', + title: 'Object Option', + description: 'Object configuration', + properties: { + name: { type: 'string' }, + }, + }, + ], + }, + }, + }; + + beforeEach(() => { + mockScrollIntoView.mockClear(); + // Mock window location + delete (window as any).location; + window.location = { + hash: '', + origin: 'http://localhost', + pathname: '/', + } as any; + }); + + test('should expand properties and scroll when hash is present on mount', async () => { + // Set hash before rendering + window.location.hash = '#nestedObject-nestedString'; + + const { container } = render(); + + await waitFor(() => { + // Should expand the nested object to show nested property + const nestedObjectElement = container.querySelector( + '[data-property-key="nestedObject"]' + ); + expect(nestedObjectElement?.classList.contains('expanded')).toBe(true); + + // Should have the correct ID for the nested property + const nestedStringElement = container.querySelector( + '#nestedObject-nestedString' + ); + expect(nestedStringElement).toBeInTheDocument(); + }); + + // Should call scrollIntoView for initial page load to handle React timing issues + // where browser scrolls before target elements are rendered/expanded + await waitFor(() => { + expect(mockScrollIntoView).toHaveBeenCalledWith({ + behavior: 'smooth', + block: 'start', + }); + }); + }); + + test('should handle hash changes after mount', async () => { + const { container } = render(); + + // Simulate hash change + window.location.hash = '#basicProperty'; + + // Trigger hashchange event + const hashChangeEvent = new Event('hashchange'); + window.dispatchEvent(hashChangeEvent); + + await waitFor(() => { + // Should set the focused property + const basicPropertyElement = container.querySelector( + '[data-property-key="basicProperty"]' + ); + expect(basicPropertyElement?.classList.contains('focused')).toBe(true); + }); + + // For hash changes after mount, we now also use manual scrolling for consistency + await waitFor(() => { + expect(mockScrollIntoView).toHaveBeenCalledWith({ + behavior: 'smooth', + block: 'start', + }); + }); + }); + + test('should handle oneOf property anchors correctly', async () => { + // Set hash for oneOf property with selection + window.location.hash = '#oneOfProperty-oneOf-1'; + + const { container } = render(); + + await waitFor(() => { + // Should expand the oneOf property + const oneOfElement = container.querySelector( + '[data-property-key="oneOfProperty"]' + ); + expect(oneOfElement?.classList.contains('expanded')).toBe(true); + + // Should have hidden anchor for oneOf selection + const oneOfAnchor = container.querySelector('#oneOfProperty-oneOf-1'); + expect(oneOfAnchor).toBeInTheDocument(); + expect(oneOfAnchor).toHaveStyle({ visibility: 'hidden' }); + }); + }); + + test('should generate correct link hrefs', async () => { + const { container } = render(); + + await waitFor(() => { + // Check basic property link + const basicPropertyLink = container.querySelector( + '[data-property-key="basicProperty"] .link-button' + ); + expect(basicPropertyLink).toHaveAttribute( + 'href', + 'http://localhost/#basicProperty' + ); + + // Check nested property link (after expansion) + const nestedObjectExpandButton = container.querySelector( + '[data-property-key="nestedObject"] .expand-button' + ); + if (nestedObjectExpandButton) { + fireEvent.click(nestedObjectExpandButton); + } + }); + + await waitFor(() => { + const nestedStringLink = container.querySelector( + '[data-property-key="nestedObject.nestedString"] .link-button' + ); + expect(nestedStringLink).toHaveAttribute( + 'href', + 'http://localhost/#nestedObject-nestedString' + ); + }); + }); + + test('should preserve native browser behavior on link clicks', async () => { + const { container } = render(); + + await waitFor(() => { + const basicPropertyLink = container.querySelector( + '[data-property-key="basicProperty"] .link-button' + ); + expect(basicPropertyLink).toBeInTheDocument(); + }); + + const basicPropertyLink = container.querySelector( + '[data-property-key="basicProperty"] .link-button' + ) as HTMLAnchorElement; + + // Simulate clicking the link + fireEvent.click(basicPropertyLink); + + // The click should not prevent default behavior + // The setTimeout in the handler should allow native navigation to occur first + await waitFor(() => { + // Property should eventually become focused + const basicPropertyElement = container.querySelector( + '[data-property-key="basicProperty"]' + ); + expect(basicPropertyElement?.classList.contains('focused')).toBe(true); + }); + }); + + test('should handle middle-click and cmd+click without interference', async () => { + const { container } = render(); + + await waitFor(() => { + const basicPropertyLink = container.querySelector( + '[data-property-key="basicProperty"] .link-button' + ); + expect(basicPropertyLink).toBeInTheDocument(); + }); + + const basicPropertyLink = container.querySelector( + '[data-property-key="basicProperty"] .link-button' + ) as HTMLAnchorElement; + + // Test middle-click (should not trigger our handler) + fireEvent.click(basicPropertyLink, { button: 1 }); + + await waitFor(() => { + // Property should not become focused with middle-click + const basicPropertyElement = container.querySelector( + '[data-property-key="basicProperty"]' + ); + expect(basicPropertyElement?.classList.contains('focused')).toBe(false); + }); + + // Test cmd+click (should not trigger our handler) + fireEvent.click(basicPropertyLink, { metaKey: true }); + + await waitFor(() => { + // Property should not become focused with cmd+click + const basicPropertyElement = container.querySelector( + '[data-property-key="basicProperty"]' + ); + expect(basicPropertyElement?.classList.contains('focused')).toBe(false); + }); + }); + + test('should convert between hash format and property key correctly', async () => { + // Test pattern properties and nested paths + const testCases = [ + { hash: '#basic-property', propertyKey: 'basic.property' }, + { hash: '#sdk-(pattern-0)', propertyKey: 'sdk.(pattern-0)' }, + { + hash: '#nested-object-child-property', + propertyKey: 'nested.object.child.property', + }, + { + hash: '#sdk-(pattern-0)-dependencies', + propertyKey: 'sdk.(pattern-0).dependencies', + }, + { hash: '#oneOfProperty-oneOf-1', propertyKey: 'oneOfProperty.oneOf.1' }, + ]; + + // Import utils to test conversion functions + const { hashToPropertyKey, propertyKeyToHash } = await import('../utils'); + + testCases.forEach(({ hash, propertyKey }) => { + expect(hashToPropertyKey(hash)).toBe(propertyKey); + expect(propertyKeyToHash(propertyKey)).toBe(hash.substring(1)); // Remove # for comparison + }); + }); + + test('should cleanup event listeners on unmount', async () => { + const removeEventListenerSpy = vi.spyOn(window, 'removeEventListener'); + + const { unmount } = render(); + + unmount(); + + expect(removeEventListenerSpy).toHaveBeenCalledWith( + 'hashchange', + expect.any(Function) + ); + }); + + test('should handle anchor links correctly', async () => { + const { container } = render(); + + await waitFor(() => { + const basicPropertyLink = container.querySelector( + '[data-property-key="basicProperty"] .link-button' + ); + expect(basicPropertyLink).toBeInTheDocument(); + }); + + const basicPropertyLink = container.querySelector( + '[data-property-key="basicProperty"] .link-button' + ) as HTMLAnchorElement; + + // Test that the link has the correct href for native browser navigation + expect(basicPropertyLink).toHaveAttribute( + 'href', + 'http://localhost/#basicProperty' + ); + + // Test that clicking the link focuses the property + fireEvent.click(basicPropertyLink); + + // The property should eventually become focused + await waitFor( + () => { + const basicPropertyElement = container.querySelector( + '[data-property-key="basicProperty"]' + ); + expect(basicPropertyElement?.classList.contains('focused')).toBe(true); + }, + { timeout: 100 } + ); + + // Verify the property has the correct ID for anchor targeting + const basicPropertyElement = container.querySelector('#basicProperty'); + expect(basicPropertyElement).toBeInTheDocument(); + expect(basicPropertyElement).toHaveAttribute('id', 'basicProperty'); + }); +}); diff --git a/src/property/PropertyField.tsx b/src/property/PropertyField.tsx index c481a24..7e1b88d 100644 --- a/src/property/PropertyField.tsx +++ b/src/property/PropertyField.tsx @@ -183,12 +183,16 @@ const PropertyField: React.FC = ({ return; // Let browser handle navigation } - // Set this property as focused when clicking link - onFocusChange?.(propertyKey); - // Expand the field if it's not already expanded - if (collapsible && hasValidSchema && !state.expanded) { - onToggle(); - } + // Don't prevent default - let browser handle navigation first + // Use setTimeout to allow browser navigation to complete before our updates + setTimeout(() => { + // Set this property as focused when clicking link + onFocusChange?.(propertyKey); + // Expand the field if it's not already expanded + if (collapsible && hasValidSchema && !state.expanded) { + onToggle(); + } + }, 0); }, [ onFocusChange, diff --git a/src/property/PropertyRow.tsx b/src/property/PropertyRow.tsx index ad090a7..e55a685 100644 --- a/src/property/PropertyRow.tsx +++ b/src/property/PropertyRow.tsx @@ -393,12 +393,16 @@ const PropertyRow: React.FC = ({ return; // Let browser handle navigation } - // Set this property as focused when clicking link - onFocusChange?.(propertyKey); - // Expand the field if it's not already expanded - if (collapsible && hasValidSchema && !state.expanded) { - onToggle(); - } + // Don't prevent default - let browser handle navigation first + // Use setTimeout to allow browser navigation to complete before our updates + setTimeout(() => { + // Set this property as focused when clicking link + onFocusChange?.(propertyKey); + // Expand the field if it's not already expanded + if (collapsible && hasValidSchema && !state.expanded) { + onToggle(); + } + }, 0); }, [ onFocusChange, From 0249cffd9a3863251a34fd1a7d7d55b3d7ae3a51 Mon Sep 17 00:00:00 2001 From: Daniel Spofford <868014+danielspofford@users.noreply.github.com> Date: Sun, 21 Sep 2025 23:05:34 -0600 Subject: [PATCH 2/2] fix anchor scrolling --- src/DeckardSchema.tsx | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/DeckardSchema.tsx b/src/DeckardSchema.tsx index 49d5670..f5f7b10 100644 --- a/src/DeckardSchema.tsx +++ b/src/DeckardSchema.tsx @@ -499,8 +499,6 @@ export const DeckardSchema: React.FC = ({ // Handle URL hash navigation and hash changes useEffect(() => { - let isInitialLoad = true; - const handleHashNavigation = () => { const hash = typeof window !== 'undefined' ? window.location.hash : ''; if (hash) { @@ -545,7 +543,6 @@ export const DeckardSchema: React.FC = ({ // Handle initial hash on mount handleHashNavigation(); - isInitialLoad = false; // Listen for hash changes if (typeof window !== 'undefined') {