-
Notifications
You must be signed in to change notification settings - Fork 2.4k
feat(materials): dynamic library registry, picker source tabs, Other category #525
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
1007b4e
5e8b63a
2e419f6
87d8d18
fd087fa
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,10 +4,14 @@ import { | |
| MaterialTarget as MaterialTargetSchema, | ||
| } from './schema/material' | ||
|
|
||
| export type MaterialSource = 'pascal' | 'community' | 'mine' | 'workspace' | ||
|
|
||
| export type MaterialCatalogItem = { | ||
| id: string | ||
| label: string | ||
| category: MaterialCategory | ||
| /** Origin of the entry. Absent = 'pascal' (all static catalog entries). */ | ||
| source?: MaterialSource | ||
| /** | ||
| * Where this finish is appropriate. Absent = universal (e.g. flat colors). | ||
| * The paint picker may filter by the slot being painted; v1 shows everything. | ||
|
|
@@ -69,6 +73,7 @@ export const MATERIAL_CATEGORIES = [ | |
| 'roofing', | ||
| 'ground', | ||
| 'glass', | ||
| 'other', | ||
| ] as const | ||
| export type MaterialCategory = (typeof MATERIAL_CATEGORIES)[number] | ||
|
|
||
|
|
@@ -4149,13 +4154,64 @@ export const MATERIAL_CATALOG: MaterialCatalogItem[] = [ | |
| }, | ||
| ] | ||
|
|
||
| const STATIC_CATALOG_IDS = new Set(MATERIAL_CATALOG.map((item) => item.id)) | ||
|
|
||
| // Embedder-registered library materials (user/community/workspace). Core stays | ||
| // passive: hosts push entries in; nothing here fetches. Static catalog entries | ||
| // win on id collision so a registration can never shadow a built-in. | ||
| const dynamicLibraryMaterials = new Map<string, MaterialCatalogItem>() | ||
| const dynamicLibraryListeners = new Set<() => void>() | ||
| let dynamicLibraryVersion = 0 | ||
|
|
||
| function notifyDynamicLibraryChange(): void { | ||
| dynamicLibraryVersion += 1 | ||
| for (const listener of [...dynamicLibraryListeners]) { | ||
| listener() | ||
| } | ||
| } | ||
|
|
||
| export function registerLibraryMaterials(items: MaterialCatalogItem[]): void { | ||
| if (items.length === 0) return | ||
| for (const item of items) { | ||
| dynamicLibraryMaterials.set(item.id, item) | ||
| } | ||
| notifyDynamicLibraryChange() | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Library refs cached as staticHigh Severity
Additional Locations (1)Reviewed by Cursor Bugbot for commit 87d8d18. Configure here. |
||
| } | ||
|
|
||
| export function unregisterLibraryMaterials(ids: string[]): void { | ||
| let changed = false | ||
| for (const id of ids) { | ||
| changed = dynamicLibraryMaterials.delete(id) || changed | ||
| } | ||
| if (changed) notifyDynamicLibraryChange() | ||
| } | ||
|
|
||
| export function getDynamicLibraryMaterials(): MaterialCatalogItem[] { | ||
| return [...dynamicLibraryMaterials.values()] | ||
| } | ||
|
|
||
| export function subscribeLibraryMaterials(listener: () => void): () => void { | ||
| dynamicLibraryListeners.add(listener) | ||
| return () => { | ||
| dynamicLibraryListeners.delete(listener) | ||
| } | ||
| } | ||
|
|
||
| export function getLibraryMaterialsVersion(): number { | ||
| return dynamicLibraryVersion | ||
| } | ||
|
|
||
| export function getMaterialsForCategory(category: MaterialCategory): MaterialCatalogItem[] { | ||
| return MATERIAL_CATALOG.filter((item) => item.category === category) | ||
| const items = MATERIAL_CATALOG.filter((item) => item.category === category) | ||
| for (const item of dynamicLibraryMaterials.values()) { | ||
| if (item.category === category && !STATIC_CATALOG_IDS.has(item.id)) items.push(item) | ||
| } | ||
| return items | ||
| } | ||
|
|
||
| export function getCatalogMaterialById(id?: string): MaterialCatalogItem | undefined { | ||
| if (!id) return undefined | ||
| return MATERIAL_CATALOG.find((item) => item.id === id) | ||
| return MATERIAL_CATALOG.find((item) => item.id === id) ?? dynamicLibraryMaterials.get(id) | ||
| } | ||
|
|
||
| export const LIBRARY_MATERIAL_REF_PREFIX = 'library:' | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,44 +2,82 @@ | |
|
|
||
| import { | ||
| getCatalogMaterialById, | ||
| getDynamicLibraryMaterials, | ||
| getLibraryMaterialIdFromRef, | ||
| getLibraryMaterialsVersion, | ||
| getMaterialsForCategory, | ||
| MATERIAL_CATEGORIES, | ||
| type MaterialCatalogItem, | ||
| type MaterialSource, | ||
| type MaterialTarget, | ||
| subscribeLibraryMaterials, | ||
| toLibraryMaterialRef, | ||
| } from '@pascal-app/core' | ||
| import { useEffect, useState } from 'react' | ||
| import { Plus } from 'lucide-react' | ||
| import { useEffect, useMemo, useState, useSyncExternalStore } from 'react' | ||
| import { triggerSFX } from '../../../lib/sfx-bus' | ||
|
|
||
| type MaterialPickerProps = { | ||
| export type MaterialSourceFilter = 'all' | MaterialSource | ||
|
|
||
| export type MaterialPickerProps = { | ||
| selectedMaterialPreset?: string | ||
| onSelectMaterialPreset?: (materialPreset: string) => void | ||
| disabled?: boolean | ||
| nodeType?: MaterialTarget | ||
| hideSideControl?: boolean | ||
| onCreateMaterialRequest?: () => void | ||
| } | ||
|
|
||
| const SOURCE_FILTERS: { id: MaterialSourceFilter; label: string }[] = [ | ||
| { id: 'all', label: 'All' }, | ||
| { id: 'pascal', label: 'Pascal' }, | ||
| { id: 'mine', label: 'Mine' }, | ||
| { id: 'workspace', label: 'Workspace' }, | ||
| { id: 'community', label: 'Community' }, | ||
| ] | ||
|
|
||
| function getCategoryLabel(category: (typeof MATERIAL_CATEGORIES)[number]) { | ||
| return category.charAt(0).toUpperCase() + category.slice(1) | ||
| } | ||
|
|
||
| function filterBySource(items: MaterialCatalogItem[], filter: MaterialSourceFilter) { | ||
| if (filter === 'all') return items | ||
| return items.filter((item) => (item.source ?? 'pascal') === filter) | ||
| } | ||
|
|
||
| /** | ||
| * Catalog material picker: a fixed row of category tabs over a scrollable grid | ||
| * of swatches. Custom-material creation lives in the scene-material section | ||
| * (the host's `+` action), not here, so it's available from any category. | ||
| * Catalog material picker: a fixed row of category tabs and a source filter row | ||
| * over a scrollable grid of swatches. Scene-material creation lives in the | ||
| * scene-material section (the host's `+` action); `onCreateMaterialRequest` is | ||
| * the host's entry point for authoring a new *library* material. | ||
| */ | ||
| export function MaterialPicker({ | ||
| selectedMaterialPreset, | ||
| onSelectMaterialPreset, | ||
| disabled = false, | ||
| onCreateMaterialRequest, | ||
| }: MaterialPickerProps) { | ||
| const [selectedCategory, setSelectedCategory] = useState<(typeof MATERIAL_CATEGORIES)[number]>( | ||
| MATERIAL_CATEGORIES[0], | ||
| ) | ||
| const [sourceFilter, setSourceFilter] = useState<MaterialSourceFilter>('all') | ||
| // Version counter so host registrations/unregistrations re-render the picker. | ||
| const libraryVersion = useSyncExternalStore( | ||
| subscribeLibraryMaterials, | ||
| getLibraryMaterialsVersion, | ||
| getLibraryMaterialsVersion, | ||
| ) | ||
| const hasWorkspaceMaterials = useMemo( | ||
| () => getDynamicLibraryMaterials().some((item) => item.source === 'workspace'), | ||
| [libraryVersion], | ||
| ) | ||
| const visibleSourceFilters = SOURCE_FILTERS.filter( | ||
| (filter) => filter.id !== 'workspace' || hasWorkspaceMaterials, | ||
| ) | ||
| const availableCategories = MATERIAL_CATEGORIES.filter( | ||
| (category) => getMaterialsForCategory(category).length > 0, | ||
| ) | ||
| const catalogItems = getMaterialsForCategory(selectedCategory) | ||
| const catalogItems = filterBySource(getMaterialsForCategory(selectedCategory), sourceFilter) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Category tabs ignore source filterMedium Severity The material picker can show an empty grid and a broken UI state when the selected category or source filter no longer has any materials. This happens because Reviewed by Cursor Bugbot for commit 2e419f6. Configure here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Category sync ignores library updatesMedium Severity The effect that syncs Additional Locations (1)Reviewed by Cursor Bugbot for commit 2e419f6. Configure here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Brush survives material unregisterMedium Severity Unregistering a library material leaves Additional Locations (2)Reviewed by Cursor Bugbot for commit 87d8d18. Configure here.
cursor[bot] marked this conversation as resolved.
|
||
|
|
||
| // Keep the visible category in sync with the externally-selected catalog | ||
| // material (a `scene:` ref matches no catalog entry, so the tab stays put). | ||
|
|
@@ -72,7 +110,7 @@ export function MaterialPicker({ | |
| setSelectedCategory(category) | ||
| // Auto-select the first material in the category so the brush is | ||
| // immediately ready (and the swatch shows as selected). | ||
| const first = getMaterialsForCategory(category)[0] | ||
| const first = filterBySource(getMaterialsForCategory(category), sourceFilter)[0] | ||
| if (first) handleCatalogSelect(first.id) | ||
| }} | ||
| type="button" | ||
|
|
@@ -81,11 +119,52 @@ export function MaterialPicker({ | |
| </button> | ||
| ))} | ||
| </div> | ||
| {/* Fixed source filter tabs — underline style, matching the catalog | ||
| browse surfaces (Items / Rooms / Build / Search) rather than the | ||
| pill-button category row above. */} | ||
| <div className="flex shrink-0 items-center gap-4 px-1"> | ||
| {visibleSourceFilters.map((filter) => ( | ||
| <button | ||
| className={`-mb-px border-b-2 px-0.5 py-1.5 font-medium text-xs transition-colors ${ | ||
| sourceFilter === filter.id | ||
| ? 'border-foreground text-foreground' | ||
| : 'border-transparent text-muted-foreground hover:text-foreground' | ||
| }`} | ||
| key={filter.id} | ||
| onClick={() => { | ||
| triggerSFX('sfx:menu-click') | ||
| setSourceFilter(filter.id) | ||
| }} | ||
| onMouseEnter={() => triggerSFX('sfx:menu-hover')} | ||
| type="button" | ||
| > | ||
| {filter.label} | ||
| </button> | ||
| ))} | ||
| </div> | ||
| {/* The only scrolling region. */} | ||
| <div | ||
| className="subtle-scrollbar grid min-h-0 flex-1 auto-rows-min gap-2 overflow-y-auto pb-1" | ||
| style={{ gridTemplateColumns: 'repeat(auto-fill, minmax(72px, 1fr))' }} | ||
| > | ||
| {onCreateMaterialRequest ? ( | ||
| <button | ||
| className="group relative flex flex-col gap-1.5 rounded-xl p-1.5 transition-colors hover:cursor-pointer hover:bg-sidebar-accent" | ||
| onClick={() => { | ||
| triggerSFX('sfx:menu-click') | ||
| onCreateMaterialRequest() | ||
| }} | ||
| onMouseEnter={() => triggerSFX('sfx:menu-hover')} | ||
| type="button" | ||
| > | ||
| <div className="flex aspect-square w-full items-center justify-center rounded-lg border border-border/45 border-dashed"> | ||
| <Plus className="size-5 text-muted-foreground group-hover:text-foreground" /> | ||
| </div> | ||
| <span className="truncate px-0.5 text-left font-medium text-[11px] text-muted-foreground group-hover:text-foreground"> | ||
| New material | ||
| </span> | ||
| </button> | ||
| ) : null} | ||
| {catalogItems.map((item) => { | ||
| const isSelected = selectedMaterialPreset === toLibraryMaterialRef(item.id) | ||
| return ( | ||
|
|
||


There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Static id collision ghost tab
Low Severity
registerLibraryMaterialsstores dynamic materials even when their IDs collide with static catalog items. While some getters, likegetMaterialsForCategory, filter these collisions,getDynamicLibraryMaterialsreturns the unfiltered list. This can result in asource: 'workspace'material being registered but not displayed in the UI, potentially showing an empty Workspace tab.Reviewed by Cursor Bugbot for commit 87d8d18. Configure here.