-
Notifications
You must be signed in to change notification settings - Fork 607
fix: keyboard shortcut hijack issues #3115
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
17 commits
Select commit
Hold shift + click to select a range
7804879
fix: logs page keyboard issues
ogzhanolguncu 223ad33
fix: other keyboard issues
ogzhanolguncu 8a38b90
chore: cleanup commennts
ogzhanolguncu 1f07540
tests: add tests coverage for shortcuts
ogzhanolguncu e278a78
Merge branch 'main' into fix-keyboard-hijack
ogzhanolguncu c9b19c8
fix: inner option key controllers
ogzhanolguncu 1ad6874
Merge branch 'main' into fix-keyboard-hijack
chronark 9b4e278
Merge branch 'main' into fix-keyboard-hijack
ogzhanolguncu 1ee0e6f
fix: focus issue of checkbox
ogzhanolguncu 6acd449
Merge branch 'main' of github.com:unkeyed/unkey into fix-keyboard-hijack
ogzhanolguncu 3434665
[autofix.ci] apply automated fixes
autofix-ci[bot] dd097a6
fix: conflict and regression
ogzhanolguncu d0f4630
Merge branch 'fix-keyboard-hijack' of github.com:unkeyed/unkey into f…
ogzhanolguncu 70105bd
fix: regression
ogzhanolguncu 88f1963
fix: revert regressions
ogzhanolguncu 0f20f4f
fix: regression
ogzhanolguncu 1b63e07
fix: wording
ogzhanolguncu 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
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
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
171 changes: 171 additions & 0 deletions
171
apps/dashboard/components/logs/checkbox/filter-item.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,171 @@ | ||
| import { KeyboardButton } from "@/components/keyboard-button"; | ||
| import { Drover } from "@/components/ui/drover"; | ||
| import { CaretRight } from "@unkey/icons"; | ||
| import { Button } from "@unkey/ui"; | ||
| import { cn } from "@unkey/ui/src/lib/utils"; | ||
| import type React from "react"; | ||
| import { type KeyboardEvent, useCallback, useEffect, useRef, useState } from "react"; | ||
|
|
||
| export type FilterItemConfig = { | ||
| id: string; | ||
| label: string; | ||
| shortcut?: string; | ||
| component: React.ReactNode; | ||
| }; | ||
|
|
||
| type FilterItemProps = FilterItemConfig & { | ||
| isFocused?: boolean; // Highlighted in the main list? | ||
| isActive?: boolean; // Is this item's popover the active one? | ||
| filterCount: number; | ||
| setActiveFilter: (id: string | null) => void; | ||
| }; | ||
|
|
||
| export const FilterItem = ({ | ||
| id, | ||
| label, | ||
| shortcut, | ||
| component, | ||
| isFocused, | ||
| isActive, | ||
| filterCount, | ||
| setActiveFilter, | ||
| }: FilterItemProps) => { | ||
| // Internal open state, primarily controlled by 'isActive' prop effect | ||
| const [open, setOpen] = useState(isActive ?? false); | ||
| const itemRef = useRef<HTMLDivElement>(null); // Ref for the trigger div | ||
| const contentRef = useRef<HTMLDivElement>(null); // Ref for the DroverContent | ||
|
|
||
| // Synchronize internal open state with the parent's isActive prop | ||
| useEffect(() => { | ||
| setOpen(isActive ?? false); | ||
| }, [isActive]); | ||
|
|
||
| // Focus the trigger div when parent indicates it's focused in the main list | ||
| // biome-ignore lint/correctness/useExhaustiveDependencies: no need to react for label | ||
| useEffect(() => { | ||
| if (isFocused && !isActive && itemRef.current) { | ||
| // Only focus trigger if not active | ||
| itemRef.current.focus({ preventScroll: true }); | ||
| } | ||
| }, [isFocused, isActive, label]); // Depend on isActive too | ||
|
|
||
| // Focus content when drover becomes active and open | ||
| // biome-ignore lint/correctness/useExhaustiveDependencies: no need to react for label | ||
| useEffect(() => { | ||
| if (isActive && open && contentRef.current) { | ||
| // Find and focus the first focusable element within the content | ||
| const focusableElements = contentRef.current.querySelectorAll<HTMLElement>( | ||
| 'button, [href], input:not([type="hidden"]), select, textarea, [tabindex]:not([tabindex="-1"])', | ||
| ); | ||
| if (focusableElements.length > 0) { | ||
| focusableElements[0].focus({ preventScroll: true }); | ||
| } else { | ||
| // Fallback: focus the content container itself if nothing else is focusable | ||
| contentRef.current.focus({ preventScroll: true }); | ||
| } | ||
| } | ||
| }, [isActive, open, label]); // Depend on isActive and open | ||
|
|
||
| const handleItemDroverKeyDown = useCallback( | ||
| (e: KeyboardEvent) => { | ||
| // No need to check isInputFocused here as parent handles ArrowLeft navigation back | ||
| // We only care about Escape to close *this* drover. | ||
| if (e.key === "Escape") { | ||
| e.preventDefault(); | ||
| e.stopPropagation(); // Stop Escape from bubbling further (e.g., closing main drover) | ||
| // Request parent to deactivate this filter and focus the trigger | ||
| setActiveFilter(null); | ||
| // Focus should return naturally because parent will set isFocused=true | ||
| // based on lastFocusedIndex after setActiveFilter(null) is processed. | ||
| } | ||
| // Allow other keys (like arrows in inputs) to behave normally | ||
| }, | ||
| [setActiveFilter], // Depend on the callback from parent | ||
| ); | ||
|
|
||
| // Handler for Drover's open state changes (e.g., clicking outside) | ||
| const handleOpenChange = useCallback( | ||
| (newOpenState: boolean) => { | ||
| // This function is called when the drover intends to close | ||
| // (e.g., click outside, Escape press handled internally if not stopped) | ||
| setOpen(newOpenState); // Keep internal state synced | ||
|
|
||
| // If the drover closed AND the parent still thinks it's active, | ||
| // we MUST inform the parent to update its state. | ||
| if (!newOpenState && isActive) { | ||
| setActiveFilter(null); | ||
| } | ||
| // If it opened via interaction (shouldn't happen if controlled), | ||
| // or closed when parent already knew, do nothing extra. | ||
| }, | ||
| [isActive, setActiveFilter], | ||
| ); | ||
|
|
||
| // Handler for clicking the trigger element | ||
| const handleTriggerClick = useCallback(() => { | ||
| // Toggle activation by telling the parent | ||
| setActiveFilter(isActive ? null : id); | ||
| }, [isActive, id, setActiveFilter]); | ||
|
|
||
| return ( | ||
| <Drover.Nested open={open} onOpenChange={handleOpenChange}> | ||
| <Drover.Trigger asChild> | ||
| {/* biome-ignore lint/a11y/useKeyWithClickEvents: <explanation> */} | ||
| <div | ||
| ref={itemRef} | ||
| className={cn( | ||
| "flex w-full items-center px-2 py-1.5 justify-between rounded-lg group cursor-pointer", | ||
| "hover:bg-gray-3 data-[state=open]:bg-gray-3", | ||
| "focus:outline-none focus:ring-2 focus:ring-accent-7", | ||
| isFocused && !isActive ? "bg-gray-4" : "", | ||
| isActive ? "bg-gray-3" : "", | ||
| )} | ||
| tabIndex={-1} | ||
| role="menuitem" | ||
| aria-haspopup="true" | ||
| aria-expanded={open} | ||
| onClick={handleTriggerClick} | ||
ogzhanolguncu marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| > | ||
| <div className="flex gap-2 items-center pointer-events-none"> | ||
| {shortcut && ( | ||
| <KeyboardButton | ||
| shortcut={shortcut} | ||
| role="presentation" | ||
| aria-haspopup="true" | ||
| title={`Press '${shortcut?.toUpperCase()}' to toggle ${label} options`} | ||
| /> | ||
| )} | ||
ogzhanolguncu marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| <span className="text-[13px] text-accent-12 font-medium select-none">{label}</span> | ||
| </div> | ||
| <div className="flex items-center gap-1.5 pointer-events-none"> | ||
| {filterCount > 0 && ( | ||
| <div className="bg-gray-6 rounded size-4 text-[11px] font-medium text-accent-12 text-center flex items-center justify-center"> | ||
| {filterCount} | ||
| </div> | ||
| )} | ||
| <Button | ||
| variant="ghost" | ||
| size="icon" | ||
| tabIndex={-1} // Non-interactive button | ||
| className="size-5 [&_svg]:size-2" | ||
| aria-hidden="true" | ||
| > | ||
| <CaretRight className="text-gray-7 group-hover:text-gray-10" /> | ||
| </Button> | ||
| </div> | ||
| </div> | ||
| </Drover.Trigger> | ||
| <Drover.Content | ||
| ref={contentRef} | ||
| className="min-w-60 w-full bg-gray-1 dark:bg-black drop-shadow-2xl p-0 border-gray-6 rounded-lg" | ||
| side="right" | ||
| align="start" | ||
| sideOffset={12} | ||
| onKeyDown={handleItemDroverKeyDown} | ||
| tabIndex={-1} | ||
| > | ||
| {component} | ||
| </Drover.Content> | ||
| </Drover.Nested> | ||
| ); | ||
| }; | ||
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.