feat(home): enable reordering and toggling home page widgets#91
feat(home): enable reordering and toggling home page widgets#91
Conversation
There was a problem hiding this comment.
Pull request overview
Adds end-user customization for the workspace home page by allowing widgets to be reordered and toggled on/off (via a “Manage widgets” modal), with persistence per workspace.
Changes:
- Introduces a home widget model (
HomeWidgetId/HomeWidget) with normalization + localStorage persistence keyed by workspace slug. - Adds a “Manage widgets” modal supporting drag-and-drop reordering and enable/disable toggles.
- Wires the PageHeader “Manage widgets” button to open the modal via a window event.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| ui/src/pages/WorkspaceHomePage.tsx | Implements widget state, persistence/normalization, modal UI, DnD reorder, and enabled filtering for rendered home sections. |
| ui/src/components/layout/PageHeader.tsx | Adds a header button click handler to open the home widgets modal via a dispatched window event. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <button | ||
| type="button" | ||
| role="switch" | ||
| aria-checked={widget.enabled} |
There was a problem hiding this comment.
The switch control has role="switch"/aria-checked but no accessible name. Screen readers will announce an unlabeled switch. Add an aria-label (e.g., including the widget label) or link it via aria-labelledby to the visible widget label text.
| aria-checked={widget.enabled} | |
| aria-checked={widget.enabled} | |
| aria-label={`Toggle ${widget.label}`} |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
nazarli-shabnam
left a comment
There was a problem hiding this comment.
@Javenn0 consider copilot warnings.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const sectionByWidgetId: Record<HomeWidgetId, ReactNode> = { | ||
| quicklinks: ( |
There was a problem hiding this comment.
sectionByWidgetId is built as Record<HomeWidgetId, ReactNode>, which eagerly evaluates all widget JSX every render (including the stickies filter/sort IIFE), even when a widget is disabled. Consider storing render functions/components (e.g. Record<HomeWidgetId, () => ReactNode>) or rendering the section inline in the .map() so disabled widgets avoid the extra work.
| const sectionByWidgetId: Record<HomeWidgetId, ReactNode> = { | |
| quicklinks: ( | |
| const sectionByWidgetId: Record<HomeWidgetId, () => ReactNode> = { | |
| quicklinks: () => ( |
| onDrop={(e) => { | ||
| e.preventDefault(); | ||
| const draggedWidgetId = e.dataTransfer.getData('text/plain'); | ||
| if ( | ||
| draggedWidgetId && | ||
| !widgets.some((candidate) => candidate.id === draggedWidgetId) | ||
| ) { | ||
| return; | ||
| } | ||
| handleWidgetDrop(widget.id); | ||
| }} |
There was a problem hiding this comment.
The drop handler reads draggedWidgetId from dataTransfer but handleWidgetDrop reorders based on the draggingWidgetId state. This can get out of sync (e.g., state update timing) and makes the dataTransfer validation effectively unused. Consider passing the dragged id into handleWidgetDrop (or deriving it solely from dataTransfer) so reordering is driven by a single source of truth.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| id={`widget-toggle-label-${widget.id}`} | ||
| className="text-sm font-medium text-(--txt-primary)" | ||
| > | ||
| {widget.label} | ||
| </span> |
There was a problem hiding this comment.
The manage-widgets switch sets both aria-label and aria-labelledby. This can result in duplicate/competing accessible names and is inconsistent with the other switches in the codebase (which use aria-labelledby). Prefer keeping just aria-labelledby (or just aria-label) so the computed name is unambiguous.
| </div> | ||
| <div className="flex items-center gap-2"> | ||
| <button | ||
| type="button" | ||
| role="switch" |
There was a problem hiding this comment.
The switch styling uses hard-coded Tailwind blues (border-blue-600 bg-blue-600). Elsewhere (e.g., SettingsPage switches) the app uses design tokens like bg-(--brand-default)/bg-(--neutral-400). Using the token classes here will keep the control consistent with theming (including dark mode) and future palette changes.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| {widgets.map((widget) => ( | ||
| <div | ||
| key={widget.id} | ||
| draggable | ||
| onDragStart={(e) => { | ||
| e.dataTransfer.setData('text/plain', widget.id); | ||
| e.dataTransfer.effectAllowed = 'move'; | ||
| setDraggingWidgetId(widget.id); | ||
| }} | ||
| onDragEnd={() => setDraggingWidgetId(null)} | ||
| onDragOver={(e) => e.preventDefault()} | ||
| onDrop={(e) => { |
There was a problem hiding this comment.
The widget reorder UI is drag-and-drop only. As implemented, there’s no keyboard-accessible way to change widget order (and HTML5 drag-and-drop is typically not usable with keyboard/screen readers), which makes this modal inaccessible for non-pointer users. Consider adding explicit “Move up / Move down” controls (or another keyboard-operable ordering mechanism) in addition to drag-and-drop, and ensure the order change is reachable via Tab/Enter/Space.
| const openFromHeader = () => setManageWidgetsOpen(true); | ||
| window.addEventListener(OPEN_HOME_WIDGETS, openFromHeader as EventListener); | ||
| return () => window.removeEventListener(OPEN_HOME_WIDGETS, openFromHeader as EventListener); |
There was a problem hiding this comment.
The event listener uses openFromHeader as EventListener casts when adding/removing the listener. This hides type mismatches and makes the handler signature unclear. Prefer defining openFromHeader with an Event parameter (even if unused) or typing it as an EventListener so the cast isn’t needed, while keeping the same function reference for removeEventListener.
| const openFromHeader = () => setManageWidgetsOpen(true); | |
| window.addEventListener(OPEN_HOME_WIDGETS, openFromHeader as EventListener); | |
| return () => window.removeEventListener(OPEN_HOME_WIDGETS, openFromHeader as EventListener); | |
| const openFromHeader = (_event: Event) => setManageWidgetsOpen(true); | |
| window.addEventListener(OPEN_HOME_WIDGETS, openFromHeader); | |
| return () => window.removeEventListener(OPEN_HOME_WIDGETS, openFromHeader); |
…plement-manage-widgets-modal-in-home-page
closes #85
This pull request introduces a customizable widget system for the workspace home page, allowing users to manage which home page widgets are visible and in what order. The changes include the ability to enable/disable widgets, reorder them via drag-and-drop, and persist these preferences per workspace using local storage. The home page UI is refactored to render widgets dynamically based on user preferences.
Home Page Widget Customization:
Manage widgets) that allows users to enable/disable widgets and reorder them via drag-and-drop, with a new icon and UI for reordering (IconGripVertical, modal implementation, drag-and-drop logic) [1] [2] [3].localStorage, and are loaded and normalized on page load [1] [2].Integration and UI Enhancements:
HomeHeaderthat triggers the widget management modal via a custom event.