fix(ui): address copilot review - race condition and init order
- commit
- f5cc89d66c4f3092bb596ffcad1cbb306335fff1
- author
- bnema · · 2025-12-27 22:55 UTC
- parent
- 5544875
- signature
- GPG Unverified
2B82207D370127A0The key is unknown
2 files · +25 -18
M internal/ui/app.go +14 -14
| @@ -1006,20 +1006,6 @@ func (a *App) initCoordinators(ctx context.Context) { | |||
| 1006 | 1006 | a.contentCoord.SetIdleInhibitor(a.deps.IdleInhibitor) | |
| 1007 | 1007 | } | |
| 1008 | 1008 | ||
| 1009 | - | // Set fullscreen callback to hide/show tab bar | |
| 1010 | - | a.contentCoord.SetOnFullscreenChanged(func(entering bool) { | |
| 1011 | - | if a.mainWindow == nil || a.mainWindow.TabBar() == nil { | |
| 1012 | - | return | |
| 1013 | - | } | |
| 1014 | - | if entering { | |
| 1015 | - | // Hide tab bar when entering fullscreen video | |
| 1016 | - | a.mainWindow.TabBar().SetVisible(false) | |
| 1017 | - | } else { | |
| 1018 | - | // Restore tab bar visibility based on normal logic | |
| 1019 | - | a.tabCoord.UpdateBarVisibility(ctx) | |
| 1020 | - | } | |
| 1021 | - | }) | |
| 1022 | - | ||
| 1023 | 1009 | // 2. Tab Coordinator | |
| 1024 | 1010 | a.tabCoord = coordinator.NewTabCoordinator(ctx, coordinator.TabCoordinatorConfig{ | |
| 1025 | 1011 | TabsUC: a.tabsUC, | |
| @@ -1049,6 +1035,20 @@ func (a *App) initCoordinators(ctx context.Context) { | |||
| 1049 | 1035 | }) | |
| 1050 | 1036 | } | |
| 1051 | 1037 | ||
| 1038 | + | // Set fullscreen callback to hide/show tab bar (after tabCoord is initialized) | |
| 1039 | + | a.contentCoord.SetOnFullscreenChanged(func(entering bool) { | |
| 1040 | + | if a.mainWindow == nil || a.mainWindow.TabBar() == nil { | |
| 1041 | + | return | |
| 1042 | + | } | |
| 1043 | + | if entering { | |
| 1044 | + | // Hide tab bar when entering fullscreen video | |
| 1045 | + | a.mainWindow.TabBar().SetVisible(false) | |
| 1046 | + | } else { | |
| 1047 | + | // Restore tab bar visibility based on normal logic | |
| 1048 | + | a.tabCoord.UpdateBarVisibility(ctx) | |
| 1049 | + | } | |
| 1050 | + | }) | |
| 1051 | + | ||
| 1052 | 1052 | // 3. Workspace Coordinator | |
| 1053 | 1053 | a.wsCoord = coordinator.NewWorkspaceCoordinator(ctx, coordinator.WorkspaceCoordinatorConfig{ | |
| 1054 | 1054 | PanesUC: a.panesUC, | |
| @@ -1006,20 +1006,6 @@ func (a *App) initCoordinators(ctx context.Context) { | |||||
| 1006 | a.contentCoord.SetIdleInhibitor(a.deps.IdleInhibitor) | 1006 | a.contentCoord.SetIdleInhibitor(a.deps.IdleInhibitor) | ||
| 1007 | } | 1007 | } | ||
| 1008 | 1008 | ||||
| 1009 | - | // Set fullscreen callback to hide/show tab bar | |||
| 1010 | - | a.contentCoord.SetOnFullscreenChanged(func(entering bool) { | |||
| 1011 | - | if a.mainWindow == nil || a.mainWindow.TabBar() == nil { | |||
| 1012 | - | return | |||
| 1013 | - | } | |||
| 1014 | - | if entering { | |||
| 1015 | - | // Hide tab bar when entering fullscreen video | |||
| 1016 | - | a.mainWindow.TabBar().SetVisible(false) | |||
| 1017 | - | } else { | |||
| 1018 | - | // Restore tab bar visibility based on normal logic | |||
| 1019 | - | a.tabCoord.UpdateBarVisibility(ctx) | |||
| 1020 | - | } | |||
| 1021 | - | }) | |||
| 1022 | - | ||||
| 1023 | // 2. Tab Coordinator | 1009 | // 2. Tab Coordinator | ||
| 1024 | a.tabCoord = coordinator.NewTabCoordinator(ctx, coordinator.TabCoordinatorConfig{ | 1010 | a.tabCoord = coordinator.NewTabCoordinator(ctx, coordinator.TabCoordinatorConfig{ | ||
| 1025 | TabsUC: a.tabsUC, | 1011 | TabsUC: a.tabsUC, | ||
| @@ -1049,6 +1035,20 @@ func (a *App) initCoordinators(ctx context.Context) { | |||||
| 1049 | }) | 1035 | }) | ||
| 1050 | } | 1036 | } | ||
| 1051 | 1037 | ||||
| 1038 | + | // Set fullscreen callback to hide/show tab bar (after tabCoord is initialized) | |||
| 1039 | + | a.contentCoord.SetOnFullscreenChanged(func(entering bool) { | |||
| 1040 | + | if a.mainWindow == nil || a.mainWindow.TabBar() == nil { | |||
| 1041 | + | return | |||
| 1042 | + | } | |||
| 1043 | + | if entering { | |||
| 1044 | + | // Hide tab bar when entering fullscreen video | |||
| 1045 | + | a.mainWindow.TabBar().SetVisible(false) | |||
| 1046 | + | } else { | |||
| 1047 | + | // Restore tab bar visibility based on normal logic | |||
| 1048 | + | a.tabCoord.UpdateBarVisibility(ctx) | |||
| 1049 | + | } | |||
| 1050 | + | }) | |||
| 1051 | + | ||||
| 1052 | // 3. Workspace Coordinator | 1052 | // 3. Workspace Coordinator | ||
| 1053 | a.wsCoord = coordinator.NewWorkspaceCoordinator(ctx, coordinator.WorkspaceCoordinatorConfig{ | 1053 | a.wsCoord = coordinator.NewWorkspaceCoordinator(ctx, coordinator.WorkspaceCoordinatorConfig{ | ||
| 1054 | PanesUC: a.panesUC, | 1054 | PanesUC: a.panesUC, | ||
M internal/ui/component/progress_bar.go +11 -4
| @@ -157,12 +157,13 @@ func (pb *ProgressBar) resetTimeout() { | |||
| 157 | 157 | // Start new timeout | |
| 158 | 158 | cb := glib.SourceFunc(func(_ uintptr) bool { | |
| 159 | 159 | pb.mu.Lock() | |
| 160 | - | // Clear timer ID before unlocking (timer is being removed) | |
| 160 | + | defer pb.mu.Unlock() | |
| 161 | + | ||
| 162 | + | // Clear timer ID (timer is being removed) | |
| 161 | 163 | pb.timeoutTimer = 0 | |
| 162 | - | pb.mu.Unlock() | |
| 163 | 164 | ||
| 164 | - | // Hide the progress bar (acquires lock internally) | |
| 165 | - | pb.Hide() | |
| 165 | + | // Hide the progress bar inline to avoid race condition | |
| 166 | + | pb.hideInternal() | |
| 166 | 167 | return false // Don't repeat | |
| 167 | 168 | }) | |
| 168 | 169 | ||
| @@ -174,6 +175,12 @@ func (pb *ProgressBar) Hide() { | |||
| 174 | 175 | pb.mu.Lock() | |
| 175 | 176 | defer pb.mu.Unlock() | |
| 176 | 177 | ||
| 178 | + | pb.hideInternal() | |
| 179 | + | } | |
| 180 | + | ||
| 181 | + | // hideInternal performs the actual hide operation. | |
| 182 | + | // Must be called with lock held. | |
| 183 | + | func (pb *ProgressBar) hideInternal() { | |
| 177 | 184 | if pb.visible { | |
| 178 | 185 | pb.visible = false | |
| 179 | 186 | pb.progressBar.SetVisible(false) | |
| @@ -157,12 +157,13 @@ func (pb *ProgressBar) resetTimeout() { | |||||
| 157 | // Start new timeout | 157 | // Start new timeout | ||
| 158 | cb := glib.SourceFunc(func(_ uintptr) bool { | 158 | cb := glib.SourceFunc(func(_ uintptr) bool { | ||
| 159 | pb.mu.Lock() | 159 | pb.mu.Lock() | ||
| 160 | - | // Clear timer ID before unlocking (timer is being removed) | 160 | + | defer pb.mu.Unlock() |
| 161 | + | ||||
| 162 | + | // Clear timer ID (timer is being removed) | |||
| 161 | pb.timeoutTimer = 0 | 163 | pb.timeoutTimer = 0 | ||
| 162 | - | pb.mu.Unlock() | |||
| 163 | 164 | ||||
| 164 | - | // Hide the progress bar (acquires lock internally) | 165 | + | // Hide the progress bar inline to avoid race condition |
| 165 | - | pb.Hide() | 166 | + | pb.hideInternal() |
| 166 | return false // Don't repeat | 167 | return false // Don't repeat | ||
| 167 | }) | 168 | }) | ||
| 168 | 169 | ||||
| @@ -174,6 +175,12 @@ func (pb *ProgressBar) Hide() { | |||||
| 174 | pb.mu.Lock() | 175 | pb.mu.Lock() | ||
| 175 | defer pb.mu.Unlock() | 176 | defer pb.mu.Unlock() | ||
| 176 | 177 | ||||
| 178 | + | pb.hideInternal() | |||
| 179 | + | } | |||
| 180 | + | ||||
| 181 | + | // hideInternal performs the actual hide operation. | |||
| 182 | + | // Must be called with lock held. | |||
| 183 | + | func (pb *ProgressBar) hideInternal() { | |||
| 177 | if pb.visible { | 184 | if pb.visible { | ||
| 178 | pb.visible = false | 185 | pb.visible = false | ||
| 179 | pb.progressBar.SetVisible(false) | 186 | pb.progressBar.SetVisible(false) | ||