Skip to content

Revert "feat: floating bar for macos by arsen"#3516

Merged
beastoin merged 1 commit intomainfrom
revert-3368-sjvrn_x
Nov 27, 2025
Merged

Revert "feat: floating bar for macos by arsen"#3516
beastoin merged 1 commit intomainfrom
revert-3368-sjvrn_x

Conversation

@beastoin
Copy link
Copy Markdown
Collaborator

@beastoin beastoin commented Nov 27, 2025

Reverts #3368

#3515

@beastoin beastoin merged commit 7664f28 into main Nov 27, 2025
1 check passed
@beastoin beastoin deleted the revert-3368-sjvrn_x branch November 27, 2025 01:57
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request reverts the 'feat: floating bar for macos' feature. The changes correctly remove the resizable window functionality and replace the parent/child window movement synchronization with a manual positioning logic. Additionally, the PR includes several beneficial UI tweaks, such as updating hardcoded colors to adaptive ones like .primary and .secondary, and changing the 'Ask AI' shortcut. I've found one area with unnecessary code duplication that should be addressed to improve maintainability.

Comment on lines +160 to 200
if #available(macOS 26.0, *) {
Button(action: onPlayPause) {
Group {
if state.isInitialising {
SpinnerView()
.frame(width: 28, height: 28)
.background(Color.white)
.clipShape(Circle())
} else {
Image(systemName: playPauseIcon)
.font(.system(size: 12, weight: .bold))
.frame(width: 28, height: 28)
.foregroundColor(.black)
.background(Color.white)
.clipShape(Circle())
.scaleEffect(state.isRecording && !state.isPaused ? 1.0 : 0.9)
}
}
}
.buttonStyle(.plain)
} else {
Button(action: onPlayPause) {
Group {
if state.isInitialising {
SpinnerView()
.frame(width: 28, height: 28)
.background(Color.white)
.clipShape(Circle())
} else {
Image(systemName: playPauseIcon)
.font(.system(size: 12, weight: .bold))
.foregroundColor(.black)
.frame(width: 28, height: 28)
.background(Color.white)
.clipShape(Circle())
.scaleEffect(state.isRecording && !state.isPaused ? 1.0 : 0.9)
}
}
}
.buttonStyle(.plain)
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The if #available(macOS 26.0, *) block introduces significant code duplication for the play/pause button. The code within the if and else branches is nearly identical, with only a minor, non-functional difference in modifier order. This duplication makes the code harder to read and maintain. It's better to remove the conditional and the duplicated code, keeping a single implementation of the button.

                Button(action: onPlayPause) {
                    Group {
                        if state.isInitialising {
                            SpinnerView()
                                .frame(width: 28, height: 28)
                                .background(Color.white)
                                .clipShape(Circle())
                        } else {
                            Image(systemName: playPauseIcon)
                                .font(.system(size: 12, weight: .bold))
                                .frame(width: 28, height: 28)
                                .foregroundColor(.black)
                                .background(Color.white)
                                .clipShape(Circle())
                                .scaleEffect(state.isRecording && !state.isPaused ? 1.0 : 0.9)
                        }
                    }
                }
                .buttonStyle(.plain)

Glucksberg pushed a commit to Glucksberg/omi-local that referenced this pull request Apr 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant