Repository navigation
feat(desktop): group sessions into tag folders in the sidebar #7037
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
Merged
Merged
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
91 changes: 91 additions & 0 deletions
91
...s/DashboardSidebar/components/DashboardSidebarGroupHeader/DashboardSidebarGroupHeader.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,91 @@ | ||
| import { cn } from "@superset/ui/utils"; | ||
| import { | ||
| type ComponentPropsWithoutRef, | ||
| forwardRef, | ||
| type ReactNode, | ||
| } from "react"; | ||
| import { HiChevronRight } from "react-icons/hi2"; | ||
|
|
||
| interface DashboardSidebarGroupHeaderProps | ||
| extends ComponentPropsWithoutRef<"div"> { | ||
| label: ReactNode; | ||
| isCollapsed: boolean; | ||
| onToggleCollapse: () => void; | ||
| actions?: ReactNode; | ||
| isEditing?: boolean; | ||
| isDraggable?: boolean; | ||
| } | ||
|
|
||
| /** Shared visual and interaction shell for every nested sidebar group. */ | ||
| export const DashboardSidebarGroupHeader = forwardRef< | ||
| HTMLDivElement, | ||
| DashboardSidebarGroupHeaderProps | ||
| >( | ||
| ( | ||
| { | ||
| label, | ||
| isCollapsed, | ||
| onToggleCollapse, | ||
| actions, | ||
| isEditing = false, | ||
| isDraggable = false, | ||
| className, | ||
| ...props | ||
| }, | ||
| ref, | ||
| ) => ( | ||
| // biome-ignore lint/a11y/noStaticElementInteractions: The header acts as a single toggle target while preserving nested inline controls in edit mode. | ||
| <div | ||
| ref={ref} | ||
| role={isEditing ? undefined : "button"} | ||
| tabIndex={isEditing ? undefined : 0} | ||
| onClick={isEditing ? undefined : onToggleCollapse} | ||
| onKeyDown={ | ||
| isEditing | ||
| ? undefined | ||
| : (event) => { | ||
| if (event.key === "Enter" || event.key === " ") { | ||
| event.preventDefault(); | ||
| onToggleCollapse(); | ||
| } | ||
| } | ||
| } | ||
| className={cn( | ||
| // Group containers have a 2px left accent border. Use a 6px left | ||
| // margin so their content aligns with borderless top-level rows. | ||
| "group ml-1.5 mr-2 flex min-h-7 items-center rounded-md py-1 pl-2 pr-2 text-[13px] font-medium text-muted-foreground transition-colors hover:bg-fill-hover", | ||
| className, | ||
| )} | ||
| {...props} | ||
| > | ||
| <div | ||
| className={cn( | ||
| "mr-1 grid h-5 w-5 shrink-0 place-items-center", | ||
| isDraggable && "cursor-grab active:cursor-grabbing", | ||
| )} | ||
| > | ||
| <HiChevronRight | ||
| className={cn( | ||
| "size-3 text-muted-foreground transition-transform duration-150", | ||
| !isCollapsed && "rotate-90", | ||
| )} | ||
| /> | ||
| </div> | ||
| <div className="flex min-w-0 flex-1 items-center gap-1.5">{label}</div> | ||
| {!isEditing && ( | ||
| <div className="ml-1 flex size-5 shrink-0 items-center justify-center"> | ||
| {actions ? ( | ||
| // biome-ignore lint/a11y/noStaticElementInteractions: Nested action controls own their semantics; this wrapper isolates them from the header toggle. | ||
| <div | ||
| className="hidden size-full items-center justify-center group-hover:flex group-has-[:focus]:flex has-[[data-state=open]]:flex" | ||
| onClick={(event) => event.stopPropagation()} | ||
| onKeyDown={(event) => event.stopPropagation()} | ||
| > | ||
| {actions} | ||
| </div> | ||
| ) : null} | ||
| </div> | ||
| )} | ||
| </div> | ||
| ), | ||
| ); | ||
1 change: 1 addition & 0 deletions
1
...ed/_dashboard/components/DashboardSidebar/components/DashboardSidebarGroupHeader/index.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| export { DashboardSidebarGroupHeader } from "./DashboardSidebarGroupHeader"; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
2 changes: 1 addition & 1 deletion
2
...components/DashboardSidebarSection/components/DashboardSidebarSectionContextMenu/types.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
106 changes: 0 additions & 106 deletions
106
...SidebarSection/components/DashboardSidebarSectionHeader/DashboardSidebarSectionHeader.tsx
This file was deleted.
Oops, something went wrong.
1 change: 0 additions & 1 deletion
1
...ebar/components/DashboardSidebarSection/components/DashboardSidebarSectionHeader/index.ts
This file was deleted.
Oops, something went wrong.
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
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.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
Repository: superset-sh/superset
Length of output: 19909
🏁 Script executed:
Repository: superset-sh/superset
Length of output: 8520
🌐 Web query:
@dnd-kit/sortable 10.0.0 useSortable listeners KeyboardSensor onKeyDown contract💡 Result:
In
@dnd-kit/sortable10.0.0, the KeyboardSensor continues to function as an abstraction layer for keyboard-driven interactions [1][2]. It does not impose a new, specific "contract" for onKeyDown in the 10.0.0 release; instead, it relies on the established event handling mechanism where the sensor monitors onKeyDown events on the focused draggable element (or its designated activator handle) [1][3][4]. Key aspects of the KeyboardSensor interaction for sortable elements include: 1. Activation: The sensor listens for onKeyDown events [1][4]. It activates when the event.key (or event.code in some configurations) matches one of the defined "start" keys (default: Space, Enter) [1][2][5]. 2. Sortable Preset Integration: To enable keyboard-based sorting (moving items via arrow keys), you must configure the KeyboardSensor with the sortableKeyboardCoordinates coordinate getter provided by@dnd-kit/sortable[6]. 3. Event Handling: By default, the sensor prevents activation if the event target is an interactive element like an input, or if it is not the draggable's assigned handle, to avoid conflicts [2][7]. This behavior is customizable via the preventActivation option in the sensor configuration [2][5][7]. 4. Focus Management: The dnd-kit library automatically manages focus during keyboard interactions [3]. When a drag starts via keyboard, focus is maintained, and it is automatically restored to the activator node when the drag ends or is canceled [3]. For implementation details, ensure your useSortable hook is within a SortableContext and that you have properly initialized the KeyboardSensor within your DndContext provider using the useSensors and useSensor hooks [6][3][8]. Version 10.0.0 itself primarily introduced changes related to independent disabling of drag/drop via a disabled object [9][10], maintaining backward compatibility for the underlying sensor architecture.Citations:
Preserve the collapse keyboard handler.
SortableSectionHeaderpassesuseSortablelisteners toDashboardSidebarGroupHeader, and the dashboard provider enablesKeyboardSensor. The final{...props}spread can replace the localonKeyDown, so Enter and Space may start a drag instead of callingonToggleCollapse. Compose both handlers or use a dedicated drag handle.🤖 Prompt for AI Agents
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.
Valid, but pre-existing rather than introduced here, so I'm leaving it out of this PR.
The component this replaces —
DashboardSidebarSection/components/DashboardSidebarSectionHeader.tsx, deleted in this PR — had the identical ordering:onKeyDownat L49 and{...props}at L64.DashboardSidebarGroupHeadercarries that behavior over verbatim, so Enter/Space on a project section header behaves exactly as it did before this PR.The underlying point stands (
useSortablelisteners includeonKeyDown, andKeyboardSensoris enabled atuseSidebarDnd.ts:319). It wants a real fix — composing both handlers or moving listeners to a dedicated drag handle — but that's a keyboard-a11y change to shared section-header behavior and belongs in its own PR rather than bundled into session grouping.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.
@Kitenite, agreed. The behavior is pre-existing and this PR preserves it during the shared-header extraction.Keeping this change out of the session-grouping PR is reasonable. The finding remains open for a separate keyboard accessibility change. Would you like me to create a follow-up GitHub issue for it?
🐇
You are interacting with an AI system.