Repository navigation
New layout playground - #869
SharonStrats wants to merge 15 commits into
Conversation
…istry@5.0.0-0) (latest: rdflib@2.4.0)
…istry@5.0.0-1) (latest: rdflib@2.4.1)
…lid-ui into new-layout-playground
…istry@5.0.0-1) (latest: rdflib@2.4.1)
|
@NoelDeMartin so here I added to the solid-ui-provider. I also extended the menu to allow me to change the font size and gap because the 3 dot menu has different styling. And oops I'm tired 😅 I just realized this is where I have moved the file explorer web components so I can use them in both solid-panes and folder-pane. Highlevel for providers we have |
NoelDeMartin
left a comment
NoelDeMartin
left a comment
There was a problem hiding this comment.
I have added a bunch of comments about specific things to improve, but in general I think it's looking good and using the Design System components correctly :).
The only thing I wonder is whether the file explorer should really be a component from solid-ui, or a dedicated pane. Either way, I think everything should be as encapsulated as possible. In general, I think it's better to expose as little public API as possible and just expose helpers or reusable functionality as necessary. Otherwise, you risk breaking dependencies if you later want to refactor something that should have been internal.
|
|
||
| let icon: HTMLElement | undefined | ||
| if (pane.name === 'profile') { | ||
| icon = dom.createElement('icon-lucide-user') |
There was a problem hiding this comment.
We should avoid using this imperative API. Instead, the icons can be rendered like this:
html`<icon-lucide-user slot="left-icon"></icon-lucide-user>`Other than that, I don't see ~icons/lucide/user being imported in this file. Components should always be imported where they are declared (same for the users icon down below, etc.).
| menuItems?: ResourceActionMenuItem[] | ||
| } | ||
|
|
||
| export function getVisibleResourceActions (options: ResourceActionsMenuOptions): ResourceActionMenuItem[] { |
There was a problem hiding this comment.
Why is this exported to the public and in a separate file? Couldn't it be just a private method in the component?
| private: boolean | ||
| } | ||
|
|
||
| export async function loadDiscoveryState (subject: NamedNode): Promise<DiscoveryState> { |
There was a problem hiding this comment.
I'm not sure why this and the resource-actions-menu.ts helpers are exported from the library, and even declared outside of the component folders. Shouldn't these be internal helpers for the file explorer components?
There was a problem hiding this comment.
I'll take a look at what you are saying. I had to have this in 2 places folder-pane and solid-panes if I remember right htat is why, but I'll see if I can do that better.
| panes: Array<PaneDefinition> = context.session.paneRegistry.list | ||
| ): Promise<Array<PaneDefinition>> { | ||
| const relevantPaneCandidates = panes.filter( | ||
| (pane) => pane.label(subject, context) && !pane.global && pane.name !== 'sharing' |
There was a problem hiding this comment.
I'm not sure this is the best way to get the relevant pane, what if a pane has a fallback label for unknown subjects such as "Unknown"? I'm not super familiar with how components are resolved, or how do we decide which pane should be used to render a certain subject. But I think it should be more robust that just looking for an empty label.
There was a problem hiding this comment.
I will have a look. I took this code how it was.
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Critical and moderate correctness, accessibility, registration, and build issues remain unresolved.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 3
Open (7)
What changed in this PR
Adds a file-explorer layout playground with reusable headers, resource actions, discovery controls, and shared RDF store context.
Changes:
- Adds file-explorer header, provider, metadata, and styling components.
- Introduces resource-action and discovery APIs.
- Updates store context, exports, typings, tests, Storybook, and dependencies.
| File | Reviewed change |
|---|---|
test/unit/index.test.ts |
Updates root export expectations. |
src/types/custom-elements.d.ts |
Adds custom-element typings. |
src/styles/theme.css |
Adds gray theme tokens. |
src/storybook/store/StorybookStore.ts |
Simplifies Storybook store creation. |
src/storybook/components/StorybookProvider.ts |
Provides the shared store context. |
src/lib/store/NoopStore.ts |
Adds a no-op store. |
src/lib/store/index.ts |
Exports store APIs. |
src/lib/store/context.ts |
Defines store context. |
src/lib/resource-actions-menu.ts |
Adds resource action bindings. |
src/lib/forms/store/StoreContext.ts |
Removes the old store context. |
src/lib/forms/store/NoopStore.ts |
Removes the old no-op store. |
src/lib/file-explorer/relevant-panes.ts |
Resolves relevant panes. |
src/lib/file-explorer/context.ts |
Extends explorer context data. |
src/lib/discovery/index.ts |
Exports discovery APIs. |
src/lib/discovery/discovery.ts |
Adds discovery state management. |
src/index.ts |
Exports new public APIs. |
src/components/resource-actions-menu/ResourceActionsMenu.ts |
Implements the resource action menu. |
src/components/resource-actions-menu/ResourceActionsMenu.styles.css |
Styles the action menu. |
src/components/resource-actions-menu/index.ts |
Exports action-menu APIs. |
src/components/resource-actions-menu/helpers.ts |
Builds menu actions. |
src/components/rdf-input/RDFInput.ts |
Migrates to shared store context. |
src/components/rdf-form/RDFForm.ts |
Migrates to shared store context. |
src/components/provider/Provider.ts |
Provides the application store. |
src/components/menu/Menu.styles.css |
Adds configurable menu width. |
src/components/menu-item/MenuItem.styles.css |
Adds configurable item styling. |
src/components/file-explorer-header/types.ts |
Defines header metadata types. |
src/components/file-explorer-header/metadata.ts |
Adds metadata utilities. |
src/components/file-explorer-header/index.ts |
Exports header components. |
src/components/file-explorer-header/FileExplorerProvider.ts |
Provides header and pane context. |
src/components/file-explorer-header/FileExplorerProvider.styles.css |
Styles the provider layout. |
src/components/file-explorer-header/FileExplorerHeaderSummary.ts |
Renders resource summaries. |
src/components/file-explorer-header/FileExplorerHeaderSummary.styles.css |
Styles resource summaries. |
src/components/file-explorer-header/FileExplorerHeaderControls.ts |
Implements header controls. |
src/components/file-explorer-header/FileExplorerHeaderControls.styles.css |
Styles header controls. |
src/components/file-explorer-header/FileExplorerHeader.ts |
Implements metadata loading and header composition. |
src/components/file-explorer-header/FileExplorerHeader.styles.css |
Styles the header container. |
package.json |
Bumps the package version. |
package-lock.json |
Updates locked dependencies and metadata. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| import 'solid-ui/components/button' | ||
| import 'solid-ui/components/menu' | ||
| import 'solid-ui/components/menu-item' |
| if (filteredPanes.length === 0) { | ||
| return relevantPaneCandidates.length > 0 ? [relevantPaneCandidates[0]] : [] |
| return firstRelevantPaneIndex < firstFilteredPaneIndex | ||
| ? [relevantPaneCandidates[0]].concat(filteredPanes) | ||
| : filteredPanes |
| const metadata = await solidLogicSingleton.resource.fetchMetadata(sym(this.fileExplorerContext.subjectUri)) | ||
| this.responseMetadata = { | ||
| modified: metadata.modified, | ||
| isPublic: metadata.access.isPublic, | ||
| canEdit: metadata.access.canEdit, | ||
| canDelete: metadata.access.canDelete, | ||
| aclUri: metadata.aclUri | ||
| } |
| if (this.paneIcon !== undefined && this._resolvedPaneIconFor !== this.paneIcon) { | ||
| this._resolvedPaneIconFor = this.paneIcon | ||
| this.resolvePaneIcon() | ||
| } |
| const subject = store.sym(this.subjectUri) | ||
| const menuItems = await this.getPaneItems(subject, this.context as DataBrowserContext, this.relevantPanes) | ||
|
|
||
| this.menuItems = menuItems |
| visibleItems.push({ | ||
| kind: 'custom', | ||
| label: options.discoverPublicly ? 'Undiscover Publicly' : 'Discover Publicly', | ||
| icon: createDiscoveryIcon('icon-lucide-globe'), | ||
| action: options.handleDiscoverPublicClick ?? (() => { | ||
| }), | ||
| }) | ||
|
|
||
| visibleItems.push({ | ||
| kind: 'custom', | ||
| label: options.discoverPrivately ? 'Undiscover Privately' : 'Discover Privately', | ||
| icon: createDiscoveryIcon('icon-lucide-lock-keyhole'), | ||
| action: options.handleDiscoverPrivateClick ?? (() => { | ||
| }), | ||
| }) |
No description provided.