Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import type {
V2WorkspaceHostType,
} from "renderer/routes/_authenticated/_dashboard/v2-workspaces/hooks/useAccessibleV2Workspaces";
import {
DEVICE_FILTER_THIS_DEVICE,
DEVICE_FILTER_ALL,
PROJECT_FILTER_ALL,
useV2WorkspacesFilterStore,
} from "renderer/routes/_authenticated/_dashboard/v2-workspaces/stores/v2WorkspacesFilterStore";
Expand Down Expand Up @@ -142,8 +142,8 @@ export function V2WorkspacesList({ workspaces }: V2WorkspacesListProps) {
);
const resetFilters = useV2WorkspacesFilterStore((state) => state.reset);

const [sortField, setSortField] = useState<SortField>("created");
const [sortDirection, setSortDirection] = useState<SortDirection>("desc");
const [sortField, setSortField] = useState<SortField>("host");
const [sortDirection, setSortDirection] = useState<SortDirection>("asc");

const handleSort = (field: SortField) => {
if (sortField === field) {
Expand All @@ -165,7 +165,7 @@ export function V2WorkspacesList({ workspaces }: V2WorkspacesListProps) {
);
const hasActiveFilters =
searchQuery.trim() !== "" ||
deviceFilter !== DEVICE_FILTER_THIS_DEVICE ||
deviceFilter !== DEVICE_FILTER_ALL ||
projectFilter !== PROJECT_FILTER_ALL;
Comment on lines 166 to 169
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Broaden the empty-state guidance.

hasActiveFilters now covers search, device, and project filters, but the empty-state copy still tells users to clear only the device filter. When the project filter is what excludes results, this points them at the wrong control.

Suggested copy tweak
- "Try a different search term or clear the device filter."
+ "Try a different search term or clear the filters."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspaces/components/V2WorkspacesList/V2WorkspacesList.tsx`
around lines 166 - 169, The empty-state message in V2WorkspacesList is still
instructing users only to clear the device filter even though hasActiveFilters
(derived from searchQuery, deviceFilter vs DEVICE_FILTER_ALL, and projectFilter
vs PROJECT_FILTER_ALL) now includes search and project filters; update the
empty-state copy to reference clearing filters more generally (e.g., “clear
filters or search terms”) or dynamically mention which filter is active so users
know to clear the device, project, or search query as appropriate; adjust the
text displayed where the empty-state is rendered in V2WorkspacesList to use the
broader copy or to inspect deviceFilter, projectFilter, and searchQuery to
tailor the suggestion.


const columnHeader = (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { Tooltip, TooltipContent, TooltipTrigger } from "@superset/ui/tooltip";
import { cn } from "@superset/ui/utils";
import { useNavigate } from "@tanstack/react-router";
import { useCallback } from "react";
import { CgLaptop } from "react-icons/cg";
import {
LuCircleCheck,
LuCircleDashed,
Expand Down Expand Up @@ -212,6 +213,17 @@ export function V2WorkspaceRow({
</div>

<span className="flex min-w-0 items-center gap-2">
{isMainWorkspace ? (
<Tooltip delayDuration={300}>
<TooltipTrigger asChild>
<CgLaptop
className="size-3.5 shrink-0 text-muted-foreground"
aria-label="Main workspace"
/>
</TooltipTrigger>
<TooltipContent side="top">Main workspace</TooltipContent>
Comment on lines 214 to +224
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Tooltip not keyboard-accessible

CgLaptop is a bare SVG with no tabIndex, so keyboard users can never focus it to trigger the tooltip. The aria-label="Main workspace" on the SVG does expose the information to screen readers, but interactive tooltip hover-on-focus is lost. Adding a tabIndex={0} to the element (or wrapping it in a focusable <span>) would allow keyboard users to access the tooltip as well.

Suggested change
<span className="flex min-w-0 items-center gap-2">
{isMainWorkspace ? (
<Tooltip delayDuration={300}>
<TooltipTrigger asChild>
<CgLaptop
className="size-3.5 shrink-0 text-muted-foreground"
aria-label="Main workspace"
/>
</TooltipTrigger>
<TooltipContent side="top">Main workspace</TooltipContent>
<CgLaptop
tabIndex={0}
className="size-3.5 shrink-0 text-muted-foreground"
aria-label="Main workspace"
/>
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspaces/components/V2WorkspacesList/components/V2WorkspaceRow/V2WorkspaceRow.tsx
Line: 214-224

Comment:
**Tooltip not keyboard-accessible**

`CgLaptop` is a bare SVG with no `tabIndex`, so keyboard users can never focus it to trigger the tooltip. The `aria-label="Main workspace"` on the SVG does expose the information to screen readers, but interactive tooltip hover-on-focus is lost. Adding a `tabIndex={0}` to the element (or wrapping it in a focusable `<span>`) would allow keyboard users to access the tooltip as well.

```suggestion
<CgLaptop
							tabIndex={0}
							className="size-3.5 shrink-0 text-muted-foreground"
							aria-label="Main workspace"
						/>
```

How can I resolve this? If you propose a fix, please make it concise.

</Tooltip>
) : null}
<span
className="min-w-0 truncate font-medium text-foreground"
title={workspace.name}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,15 @@ interface V2WorkspacesFilterState {
export const useV2WorkspacesFilterStore = create<V2WorkspacesFilterState>()(
(set) => ({
searchQuery: "",
deviceFilter: DEVICE_FILTER_THIS_DEVICE,
deviceFilter: DEVICE_FILTER_ALL,
projectFilter: PROJECT_FILTER_ALL,
setSearchQuery: (searchQuery) => set({ searchQuery }),
setDeviceFilter: (deviceFilter) => set({ deviceFilter }),
setProjectFilter: (projectFilter) => set({ projectFilter }),
reset: () =>
set({
searchQuery: "",
deviceFilter: DEVICE_FILTER_THIS_DEVICE,
deviceFilter: DEVICE_FILTER_ALL,
projectFilter: PROJECT_FILTER_ALL,
}),
}),
Expand Down
2 changes: 1 addition & 1 deletion bun.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading