-
Notifications
You must be signed in to change notification settings - Fork 16
fix: Prevent duplicate page loads on rapid keyboard shortcut use (fixes #198). #200
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
Conversation
WalkthroughThis PR modifies the Changes
Sequence Diagram(s)sequenceDiagram
participant UI as User Interface
participant SCP as StateContextProvider
participant Console as Console
UI->>SCP: Trigger navigation action
SCP->>SCP: Check UI state (READY?)
alt UI_STATE is not READY
SCP->>Console: Log warning "Page load in progress"
SCP-->>UI: Return early (skip navigation)
else
SCP->>SCP: Set UI state to READY
SCP->>Other: Continue with navigation actions
end
Assessment against linked issues
Possibly related PRs
Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🪧 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 (
|
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.
The descriptions in Conventional Messages are usually in imperative mood.
for the PR title, how about:
fix: Prevent duplicate page loads on rapid keyboard shortcut use (fixes #198).
I looked briefly Couple questions.
|
If you mean add the state check right after the first four case checks, I believe it is identical right?
Good one. |
I think if just 4 then identical, but maybe we want to block all actions. anyways we'll see what @junhaoliao thinks |
ah, I see what you mean now. I agree with that then, it would make a lot of sense for editors to check uiStates before performing any actions. |
But what the cons for this approach is that whenever uiState changes, this callback function gets re-rendered. Do we want to do that? |
It's probably fine, but if you want you can test that nothing really slow happens. Or you can try and just pass the uiState ref instead? |
|
Description
Fixes #198.
Checklist
breaking change.
Validation performed
10000
or less, open the console.Ctrl/ Cmd + [
and observe that each page gets loaded only once.Summary by CodeRabbit
Summary by CodeRabbit