-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
fix: Wait until any previous filter is saving #8752
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
WalkthroughWalkthroughThe recent changes to the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant useViewFilters
participant SaveOperation
User->>useViewFilters: Trigger save operation
alt Previous save in progress
useViewFilters-->>User: Wait until previous save completes
else No save in progress
useViewFilters->>SaveOperation: Initiate save
SaveOperation-->>useViewFilters: Confirm save completion
useViewFilters-->>User: Notify save completion
end
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 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 as PR comments)
Additionally, you can add CodeRabbit Configration 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.
Actionable comments posted: 0
Outside diff range and nitpick comments (2)
packages/nc-gui/composables/useViewFilters.ts (2)
Line range hint
132-132: Avoid using spread syntax on accumulators due to performance concerns.Consider using
.pushor.concatinstead of spread syntax in thereducecallback to avoid quadratic time complexity.Tools
Biome
[error] 354-356: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)
Unsafe fix: Omit the else clause.
Line range hint
90-95: Consider removing unnecessary else clauses to simplify control flow and enhance readability.- else { + // Removed unnecessary else clauseRemoving these clauses where the previous branches exit early (e.g., return, throw) can make the code cleaner and easier to follow.
Also applies to: 92-95, 154-156, 157-166, 163-165, 186-189
Tools
Biome
[error] 354-356: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)
Unsafe fix: Omit the else clause.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- packages/nc-gui/composables/useViewFilters.ts (1 hunks)
Additional context used
Biome
packages/nc-gui/composables/useViewFilters.ts
[error] 90-95: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)
Unsafe fix: Omit the else clause.
[error] 92-95: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)
[error] 154-156: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)
Unsafe fix: Omit the else clause.
[error] 157-166: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)
Unsafe fix: Omit the else clause.
[error] 163-165: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)
Unsafe fix: Omit the else clause.
[error] 186-189: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)
Unsafe fix: Omit the else clause.
[error] 354-356: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)
Unsafe fix: Omit the else clause.
[error] 132-132: Avoid the use of spread (
...) syntax on accumulators. (lint/performance/noAccumulatingSpread)Spread syntax should be avoided on accumulators (like those in
.reduce) because it causes a time complexity ofO(n^2).
Consider methods such as .splice or .push instead.
Additional comments not posted (1)
packages/nc-gui/composables/useViewFilters.ts (1)
353-357: Consider refactoring to avoid redundant else-if conditions.- else if (Array.from({ length: i }).some((_, index) => savingStatus[index])) { + else if (Array.from({ length: i }, (_, index) => savingStatus[index]).includes(true)) {This refactoring simplifies the condition check by directly checking if any previous filters are still saving, improving readability.
Likely invalid or redundant comment.
Tools
Biome
[error] 354-356: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)
Unsafe fix: Omit the else clause.
|
Uffizzi Preview |
Change Summary
Provide summary of changes with issue number if any.
Change type
Test/ Verification
Provide summary of changes.
Additional information / screenshots (optional)
Anything for maintainers to be made aware of