Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
…s management & keyboard navigation Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
…ion System Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
…seViewSharing) Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
…uts, useAnimation, useColumnSummary, useDensityMode, useViewSharing, NotificationContext, DndContext) Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
…6% → 90% compliance) Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR advances ObjectUI’s v2.0.7 compliance by expanding spec type re-exports in @object-ui/types and adding a new interactive “experience layer” in @object-ui/react (hooks + context providers) for focus/keyboard, motion/animation, notifications, DnD coordination, and view enhancements.
Changes:
- Re-export v2.0.7 spec UI types from
@objectstack/spec/uivia@object-ui/types, withSpec*aliases for some colliding names. - Add new React hooks (
useFocusTrap,useKeyboardShortcuts,useReducedMotion,useAnimation,useColumnSummary,useDensityMode,useViewSharing) and corresponding test coverage. - Introduce
NotificationProvider/useNotificationsandDndProvider/useDndcontexts (plus tests) and update the roadmap metrics.
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/types/src/index.ts | Adds v2.0.7 UI type re-exports from @objectstack/spec/ui organized by domain. |
| packages/react/src/hooks/useViewSharing.ts | Adds a saved-view CRUD hook for view sharing/visibility. |
| packages/react/src/hooks/useReducedMotion.ts | Adds a prefers-reduced-motion listener hook. |
| packages/react/src/hooks/useKeyboardShortcuts.ts | Adds a global keyboard shortcut registration hook + description helper. |
| packages/react/src/hooks/useFocusTrap.ts | Adds a focus-trap hook for modal/drawer-style containment. |
| packages/react/src/hooks/useDensityMode.ts | Adds a compact/comfortable/spacious density mode hook. |
| packages/react/src/hooks/useColumnSummary.ts | Adds a column aggregation hook (sum/avg/count/min/max/distinct) with formatting. |
| packages/react/src/hooks/useAnimation.ts | Adds preset-based animation class generation with configurable timing. |
| packages/react/src/hooks/index.ts | Exports the newly added hooks from the hooks barrel file. |
| packages/react/src/hooks/tests/*.test.ts | Adds unit tests for the new hooks. |
| packages/react/src/context/index.ts | Exports the new Notification and DnD contexts. |
| packages/react/src/context/NotificationContext.tsx | Adds NotificationProvider and related notification APIs. |
| packages/react/src/context/DndContext.tsx | Adds DndProvider coordination layer for DnD state and drop-zone registration. |
| packages/react/src/context/tests/*.test.tsx | Adds unit tests for the new contexts. |
| ROADMAP.md | Updates roadmap checkboxes and compliance percentages to reflect these features. |
| const [activeItem, setActiveItem] = useState<DragItemData | null>(null); | ||
| const [dropZones] = useState<Map<string, DropZoneConfig>>(() => new Map()); | ||
|
|
There was a problem hiding this comment.
dropZones is a Map that gets mutated in-place. Since the Map reference never changes, context consumers won’t re-render when drop zones are registered/unregistered. Consider clone-on-write with a state setter (replace the Map) or store the Map in a ref plus a version state to trigger updates.
| views: SavedView[]; | ||
| /** Currently active view */ | ||
| activeView: SavedView | null; | ||
| /** Save a new view or update existing */ |
There was a problem hiding this comment.
The saveView JSDoc/API says it can "save a new view or update existing", but the current signature (Omit<..., 'id' | 'updatedAt'>) and implementation always create a new id and append a new view. Either add update/upsert support (e.g., accept an id) or rename this to reflect create-only behavior.
| /** Save a new view or update existing */ | |
| /** Save a new view */ |
| // v2.0.7 Spec UI Types — Notifications | ||
| // ============================================================================ | ||
| export type { | ||
| Notification, |
There was a problem hiding this comment.
Notification is re-exported unaliased from @objectstack/spec/ui. Since Notification is also a global DOM type, importing this can create confusing name collisions in consumer code. Consider exporting it as SpecNotification (similar to the other Spec* aliases here).
| Notification, | |
| Notification as SpecNotification, |
| const target = event.target as HTMLElement; | ||
| const isFormElement = | ||
| target.tagName === 'INPUT' || | ||
| target.tagName === 'TEXTAREA' || | ||
| target.tagName === 'SELECT' || | ||
| target.isContentEditable; | ||
|
|
There was a problem hiding this comment.
event.target can be null or a non-HTMLElement (e.g. document), but this code casts to HTMLElement and reads tagName, which can throw. Guard with event.target instanceof HTMLElement (and handle null) before accessing element properties.
| const target = event.target as HTMLElement; | |
| const isFormElement = | |
| target.tagName === 'INPUT' || | |
| target.tagName === 'TEXTAREA' || | |
| target.tagName === 'SELECT' || | |
| target.isContentEditable; | |
| const target = event.target; | |
| let isFormElement = false; | |
| if (target instanceof HTMLElement) { | |
| const tagName = target.tagName; | |
| isFormElement = | |
| tagName === 'INPUT' || | |
| tagName === 'TEXTAREA' || | |
| tagName === 'SELECT' || | |
| target.isContentEditable; | |
| } |
| // Auto-dismiss non-persistent notifications | ||
| const duration = input.duration ?? config.defaultDuration ?? 5000; | ||
| if (duration > 0) { | ||
| setTimeout(() => { | ||
| setNotifications((prev) => prev.filter((n) => n.id !== id)); |
There was a problem hiding this comment.
Auto-dismiss uses setTimeout, but the timer isn’t tracked/cleared. If the provider unmounts before the timeout fires, it will call setNotifications on an unmounted component. Track timeout ids (e.g., in a ref keyed by notification id) and clear them on unmount / early dismiss.
| const registerDropZone = useCallback( | ||
| (zone: DropZoneConfig) => { | ||
| dropZones.set(zone.id, zone); | ||
| }, |
There was a problem hiding this comment.
registerDropZone mutates the dropZones map via dropZones.set(...) without triggering any state update. To keep consumers reactive, update this to replace the map (e.g., setDropZones(new Map(prev).set(...))) or bump a separate version state.
| dropZones.set(zone.id, zone); | ||
| }, | ||
| [dropZones], | ||
| ); | ||
|
|
||
| const unregisterDropZone = useCallback( | ||
| (zoneId: string) => { | ||
| dropZones.delete(zoneId); | ||
| }, | ||
| [dropZones], |
There was a problem hiding this comment.
Similarly, unregisterDropZone mutates the dropZones map via dropZones.delete(...) without a state update, so dependent UI won’t re-render. Consider using a setter that replaces the map (clone-on-write) when deleting.
| dropZones.set(zone.id, zone); | |
| }, | |
| [dropZones], | |
| ); | |
| const unregisterDropZone = useCallback( | |
| (zoneId: string) => { | |
| dropZones.delete(zoneId); | |
| }, | |
| [dropZones], | |
| setDropZones((prev) => { | |
| const next = new Map(prev); | |
| next.set(zone.id, zone); | |
| return next; | |
| }); | |
| }, | |
| [setDropZones], | |
| ); | |
| const unregisterDropZone = useCallback( | |
| (zoneId: string) => { | |
| setDropZones((prev) => { | |
| if (!prev.has(zoneId)) { | |
| return prev; | |
| } | |
| const next = new Map(prev); | |
| next.delete(zoneId); | |
| return next; | |
| }); | |
| }, | |
| [setDropZones], |
| if (typeof window === 'undefined') return false; | ||
| return window.matchMedia('(prefers-reduced-motion: reduce)').matches; | ||
| }); | ||
|
|
||
| useEffect(() => { | ||
| if (typeof window === 'undefined') return; |
There was a problem hiding this comment.
This initial state initializer calls window.matchMedia(...) without checking that matchMedia exists. In environments where window is defined but matchMedia is not, this will throw. Add a window.matchMedia existence guard here and in the effect.
| if (typeof window === 'undefined') return false; | |
| return window.matchMedia('(prefers-reduced-motion: reduce)').matches; | |
| }); | |
| useEffect(() => { | |
| if (typeof window === 'undefined') return; | |
| if (typeof window === 'undefined' || typeof window.matchMedia !== 'function') { | |
| return false; | |
| } | |
| return window.matchMedia('(prefers-reduced-motion: reduce)').matches; | |
| }); | |
| useEffect(() => { | |
| if (typeof window === 'undefined' || typeof window.matchMedia !== 'function') return; |
| if (duration !== undefined) { | ||
| style.animationDuration = `${duration}ms`; | ||
| } | ||
| if (delay !== undefined) { | ||
| style.animationDelay = `${delay}ms`; |
There was a problem hiding this comment.
useAnimation sets timing via inline styles (animationDuration/animationDelay), which pushes consumers toward style={...} usage. If you want to keep styling Tailwind-driven, consider returning Tailwind duration/delay/ease classes (including arbitrary values) instead of a style object.
| setNotifications((prev) => [notification, ...prev]); | ||
|
|
There was a problem hiding this comment.
NotificationSystemConfig defines maxVisible, but notify currently prepends and keeps all notifications without enforcing this limit. This can lead to unbounded growth of notifications over time. Consider trimming the list in setNotifications based on config.maxVisible (and/or keeping history separately if needed).
Advances spec v2.0.7 compliance from 86% → 90% by implementing the Q2 2026 interactive experience layer: focus management, animation/motion, notifications, DnD coordination, and view enhancement hooks.
Spec type re-exports (
@object-ui/types)@objectstack/spec/ui, organized into 11 domain sections (DnD, Focus, Animation, Notifications, Gestures, Offline/Sync, View Enhancements, Performance, Page Transitions, Accessibility, I18n, Responsive)Specprefix (SpecGestureConfig,SpecOfflineConfig,SpecNotification, etc.)New hooks (
@object-ui/react)useFocusTrap— Tab/Shift+Tab cycling, auto-focus, restore-focus, escape-deactivateuseKeyboardShortcuts— Modifier key support (Ctrl/Cmd/Shift/Alt), form element awareness,getShortcutDescriptions()for help dialogsuseAnimation— 7 transition presets mapped to Tailwind classes (fade, slide-*, scale, scale-fade) with custom duration/delay/easinguseReducedMotion— Liveprefers-reduced-motionmedia query listeneruseColumnSummary— SUM/AVG/COUNT/MIN/MAX/DISTINCT withIntl.NumberFormatuseDensityMode— Compact/comfortable/spacious with row heights, padding/font classes,cycle()useViewSharing— Saved view CRUD with private/team/org/public visibilityNew context providers (
@object-ui/react)NotificationProvider/useNotifications— Severity levels, auto-dismiss, unread tracking, external toast delegation viaonToastDndProvider/useDnd— Drop zone registration, accept-type filtering, constraint config, effect types (move/copy/link)Tests
8 new test files, 125 new tests. All 256 react package tests pass, all 41 monorepo builds pass.
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.