fix(taskswitch): fix window visibility issues and race condition in T…#1026
Open
zorowk wants to merge 1 commit into
Open
fix(taskswitch): fix window visibility issues and race condition in T…#1026zorowk wants to merge 1 commit into
zorowk wants to merge 1 commit into
Conversation
…askSwitcher Log: 1. Ensure workspace opacity is restored when TaskSwitcher exits, even with zero windows, to prevent permanent window transparency. 2. Fix race condition in prejudgment() by removing premature 'switchOn=false' which caused object destruction before cleanup. 3. Fix syntax error in refHeight property of TaskWindowPreview. PMS: BUG-366737 Influence: Fixes workspace window display anomalies and task switching reliability.
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: zorowk The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThis PR fixes TaskSwitcher window visibility glitches, a race condition during task switch completion, and a small syntax issue in TaskWindowPreview, primarily by tightening conditions around visibility/animation, deferring cleanup until animations are done, and explicitly restoring surface opacity/positions. Sequence diagram for TaskSwitcher prejudgment and visibility/animation handlingsequenceDiagram
actor User
participant TaskSwitcher_root as TaskSwitcher_root
participant previewWindows as previewWindows
participant switchView as switchView
User ->> TaskSwitcher_root: prejudgment()
TaskSwitcher_root ->> previewWindows: read finishedAnimations
TaskSwitcher_root ->> previewWindows: read totalAnimations
alt [previewWindows.finishedAnimations !== previewWindows.totalAnimations]
TaskSwitcher_root ->> TaskSwitcher_root: prejudgment()
else [previewWindows.finishedAnimations === previewWindows.totalAnimations]
TaskSwitcher_root ->> TaskSwitcher_root: showTask(false)
TaskSwitcher_root ->> switchView: read count
alt [root.enableAnimation && switchView.count > 0 && switchView.count <= 18]
TaskSwitcher_root ->> previewWindows: set reverse = false
TaskSwitcher_root ->> previewWindows: set model = root.model
else [!root.enableAnimation || switchView.count === 0 || switchView.count > 18]
TaskSwitcher_root ->> previewWindows: set model = null
end
end
Note over TaskSwitcher_root,previewWindows: root.switchOn is no longer set to false inside prejudgment(),
Note over TaskSwitcher_root,previewWindows: preventing premature destruction before cleanup
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- In
showTask(visible), changing the early return condition to only trigger whenvisible && switchView.count === 0meansshowTask(false)can now run withswitchView.count === 0, which may makeswitchView.currentItemundefined later in the function; consider guarding uses ofcurrentItem(or restoring an early return) when there are no items. - The new call to
prejudgment()whenfinishedAnimations !== totalAnimationschanges the previous behavior of simply returning; it might be worth double-checking thatprejudgment()is safe to call mid-animation and cannot be invoked twice for the same cycle (e.g., via both animation completion and this early path).
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `showTask(visible)`, changing the early return condition to only trigger when `visible && switchView.count === 0` means `showTask(false)` can now run with `switchView.count === 0`, which may make `switchView.currentItem` undefined later in the function; consider guarding uses of `currentItem` (or restoring an early return) when there are no items.
- The new call to `prejudgment()` when `finishedAnimations !== totalAnimations` changes the previous behavior of simply returning; it might be worth double-checking that `prejudgment()` is safe to call mid-animation and cannot be invoked twice for the same cycle (e.g., via both animation completion and this early path).Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Log:
PMS: BUG-366737
Influence: Fixes workspace window display anomalies and task switching reliability.
Summary by Sourcery
Fix TaskSwitcher window visibility handling and prevent race conditions during task switching cleanup.
Bug Fixes: