feat: add override button to ratelimits#2950
Conversation
ogzhanolguncu
requested review from
chronark,
mcstepp and
perkinsjr
as code owners
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
📝 WalkthroughWalkthroughThis pull request modifies the handling of the Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant NN as NamespaceNavbar
participant ID as IdentifierDialog
participant CB as CopyableIDButton
U->>NN: Click "Open Identifier Dialog" Button
NN->>NN: set open state to true
NN->>ID: Render IdentifierDialog (isModalOpen = true)
ID->>FormInput: Set readOnly & disabled based on identifier prop presence
U->>CB: Long-press to select text
CB->>CB: Start timer for text selection
CB->>CB: Check if text is selected
CB->>CopyButton: Trigger copy if no text selected
Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Thank you for following the naming conventions for pull request titles! 🙏 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/dashboard/components/navigation/copyable-id-button.tsx (2)
10-14: Consider adding keyboard accessibility.While the component handles mouse interactions well, it doesn't provide keyboard accessibility for the long-press text selection feature. Users who navigate with keyboards would be unable to access this functionality.
Consider adding keyboard support with something like:
export const CopyableIDButton = ({ value, className = "" }: CopyableIDButtonProps) => { const textRef = useRef<HTMLDivElement>(null); const pressTimer = useRef<NodeJS.Timeout | null>(null); const copyButtonRef = useRef<HTMLButtonElement>(null); + + const selectText = () => { + if (textRef.current) { + const range = document.createRange(); + range.selectNodeContents(textRef.current); + const selection = window.getSelection(); + if (selection) { + selection.removeAllRanges(); + selection.addRange(range); + } + } + }; + + const handleKeyDown = (e: React.KeyboardEvent) => { + if (e.key === 'Enter') { + copyButtonRef.current?.click(); + } else if (e.key === ' ' || e.key === 'Space') { + selectText(); + e.preventDefault(); + } + };
31-45: Reduce code duplication between handlers.The code for clearing the timer is duplicated in both
handleMouseUpandhandleMouseLeave. Consider extracting this logic to a separate function.const handleMouseDown = () => { // Start a long-press timer pressTimer.current = setTimeout(() => { // Rest of the code... }, 500); }; + const clearPressTimer = () => { + if (pressTimer.current) { + clearTimeout(pressTimer.current); + pressTimer.current = null; + } + }; const handleMouseUp = () => { // Clear the timer if mouse is released before long-press threshold - if (pressTimer.current) { - clearTimeout(pressTimer.current); - pressTimer.current = null; - } + clearPressTimer(); }; const handleMouseLeave = () => { // Clear the timer if mouse leaves the button - if (pressTimer.current) { - clearTimeout(pressTimer.current); - pressTimer.current = null; - } + clearPressTimer(); };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/dashboard/app/(app)/ratelimits/[namespaceId]/namespace-navbar.tsx(3 hunks)apps/dashboard/components/dashboard/copy-button.tsx(2 hunks)apps/dashboard/components/navigation/copyable-id-button.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/dashboard/app/(app)/ratelimits/[namespaceId]/namespace-navbar.tsx
- apps/dashboard/components/dashboard/copy-button.tsx
⏰ Context from checks skipped due to timeout of 90000ms (18)
- GitHub Check: Test Packages / Test ./packages/cache
- GitHub Check: Test Packages / Test ./internal/clickhouse
- GitHub Check: Test Go API Local / test_agent_local
- GitHub Check: Test Packages / Test ./packages/rbac
- GitHub Check: Test Packages / Test ./packages/hono
- GitHub Check: Test Packages / Test ./packages/api
- GitHub Check: Test Packages / Test ./packages/nextjs
- GitHub Check: Test Packages / Test ./internal/id
- GitHub Check: Test Packages / Test ./internal/keys
- GitHub Check: Test Packages / Test ./internal/hash
- GitHub Check: Test Packages / Test ./internal/billing
- GitHub Check: Test Packages / Test ./internal/encryption
- GitHub Check: Test API / API Test Local
- GitHub Check: Test Packages / Test ./internal/resend
- GitHub Check: Test Agent Local / test_agent_local
- GitHub Check: Build / Build
- GitHub Check: autofix
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (3)
apps/dashboard/components/navigation/copyable-id-button.tsx (3)
1-8: Component props look good!The component correctly defines its props with appropriate types. The
classNameprop is optional with a default value, which provides good flexibility for styling.
47-56: Good handling of click vs. long-press behavior.The click handler intelligently checks if text has been selected before deciding whether to trigger the copy action. This provides a good user experience.
58-80: Well-structured component with good UX considerations.The component is well-structured and provides a good user experience:
- Clear separation of the text and copy button
- The
select-textclass on the text container ensures users can select text- Making the CopyButton non-interactive directly with
pointer-events-noneensures the parent button's logic controls the behaviorThis creates a cohesive UI element that handles both copying and text selection.
|
@chronark @perkinsjr can I get a quick review for that? |
|
after this call |
|
:chef_kissing_fingers_intensifies: |
2 participants
What does this PR do?
Fixes # (issue)
If there is not an issue for this, please create one first. This is used to tracking purposes and also helps use understand why this PR exists
Type of change
How should this be tested?
Checklist
Required
pnpm buildpnpm fmtconsole.logsgit pull origin mainAppreciated
Summary by CodeRabbit
CopyableIDButtonfor copying the namespace identifier and a button to open the identifier dialog, ensuring smoother and more intuitive user interactions.