feat: add upstream improvements from PRs 131, 189, 186 #3
Conversation
- PR gluonfield#131: Make macOS input field scrollable when text exceeds max height - Wrap TextField in ScrollView with height tracking - Cap scrollable area at 130 points (similar to iMessage behavior) - Add ViewHeightKey preference key for dynamic height updates - PR gluonfield#189: Improve voice list in settings - Filter voices to current system language - Remove duplicate voice entries - Only show quality indicator for Enhanced/Premium voices - Use system default voice unless user explicitly overrides - PR gluonfield#186: Add iOS 18 dark mode icon support - Add dark and tinted icon variants to Contents.json - Add .gitignore with .DS_Store entry - Add placeholder icon files (replace with actual dark/tinted designs)
Summary of ChangesHello @eulicesl, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request integrates several upstream improvements focused on enhancing the user experience across different platforms. It introduces dynamic scrolling for the macOS text input field, refines the voice selection process in settings for better usability, and adds foundational support for iOS 18's dark mode and tinted app icons. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request brings in several valuable improvements from upstream, including a scrollable text input on macOS, better voice selection logic, and support for dark mode app icons on iOS 18. The changes are well-structured and align with the descriptions.
I've identified a couple of areas for improvement:
- In
SpeechService.swift, the fallback logic for voice fetching is inconsistent with the primary logic, as it doesn't remove duplicate voices. I've suggested a refactoring to make this consistent. - In
InputFields_macOS.swift, a magic number is used for the maximum text field height. Extracting this into a constant would improve maintainability.
Overall, this is a great set of enhancements. Addressing these points will make the code even more robust and maintainable.
| } | ||
| } | ||
| } | ||
| .frame(maxHeight: min(textFieldHeight, 130)) |
There was a problem hiding this comment.
The value 130 is a magic number. It's better to define it as a private constant at the top of your View struct, for example: private let maxTextFieldHeight: CGFloat = 130. Using a named constant improves readability and makes it easier to modify this value in the future.
| .frame(maxHeight: min(textFieldHeight, 130)) | |
| .frame(maxHeight: min(textFieldHeight, maxTextFieldHeight)) |
There was a problem hiding this comment.
Pull request overview
This pull request integrates three upstream improvements to enhance the user experience: a scrollable text input field for macOS that automatically expands up to a maximum height similar to iMessage, smarter voice selection in settings that filters to the system language and removes duplicates, and iOS 18 dark mode icon support with tinted variants.
- Implements height-responsive scrollable TextField for macOS with a 130-point maximum height cap
- Improves voice list by filtering to current system language, deduplicating entries, and showing quality indicators only for enhanced/premium voices
- Adds iOS 18 dark and tinted icon variants with proper asset catalog configuration
Reviewed changes
Copilot reviewed 5 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| InputFields_macOS.swift | Wraps TextField in ScrollView with dynamic height tracking using PreferenceKey to enable scrolling when text exceeds 130 points |
| Settings.swift | Updates default voice identifier initialization to use system default voice |
| SpeechService.swift | Adds system default voice detection, filters voices by system language, removes duplicates, and improves quality-based sorting |
| AVSpeechSynthesisVoice+Extension.swift | Adds displayString property to show quality labels only for enhanced/premium voices, not default voices |
| Contents.json | Adds dark and tinted appearance variants for iOS 18 icon support |
| .gitignore | Adds .DS_Store to ignore macOS system files |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
| @AppStorage("appUserInitials") private var appUserInitials: String = "" | ||
| @AppStorage("pingInterval") private var pingInterval: String = "5" | ||
| @AppStorage("voiceIdentifier") private var voiceIdentifier: String = "" | ||
| @AppStorage("voiceIdentifier") private var voiceIdentifier: String = SpeechSynthesizer.systemDefaultVoiceIdentifier() |
There was a problem hiding this comment.
The @AppStorage property wrapper requires a compile-time constant as the default value, but SpeechSynthesizer.systemDefaultVoiceIdentifier() is a function call. This will cause a compilation error.
Instead, initialize with an empty string and handle the default logic in the getter or when reading the value:
@AppStorage("voiceIdentifier") private var voiceIdentifier: String = ""Then in SpeechService.swift, the getVoiceIdentifier() method already handles the fallback to system default when the stored value is empty.
| @AppStorage("voiceIdentifier") private var voiceIdentifier: String = SpeechSynthesizer.systemDefaultVoiceIdentifier() | |
| @AppStorage("voiceIdentifier") private var voiceIdentifier: String = "" |
| ScrollViewReader { proxy in | ||
| ScrollView { | ||
| TextField("Message", text: $message.animation(.easeOut(duration: 0.3)), axis: .vertical) | ||
| .focused($isFocusedInput) | ||
| .font(.system(size: 14)) | ||
| .frame(maxWidth:.infinity, minHeight: 40) | ||
| .clipped() | ||
| .textFieldStyle(.plain) | ||
| #if os(macOS) | ||
| .onSubmit { | ||
| if NSApp.currentEvent?.modifierFlags.contains(.shift) == true { | ||
| message += "\n" | ||
| } else { | ||
| sendMessage() | ||
| } | ||
| } | ||
| .onSubmit { | ||
| if NSApp.currentEvent?.modifierFlags.contains(.shift) == true { | ||
| message += "\n" | ||
| } else { | ||
| sendMessage() | ||
| } | ||
| } | ||
| #endif | ||
| /// TextField bypasses drop area | ||
| .allowsHitTesting(!fileDropActive) | ||
| /// TextField bypasses drop area | ||
| .allowsHitTesting(!fileDropActive) | ||
| #if os(macOS) | ||
| .addCustomHotkeys(hotkeys) | ||
| .addCustomHotkeys(hotkeys) | ||
| #endif | ||
| .padding(.trailing, 80) | ||
|
|
||
|
|
||
| .padding(.trailing, 80) | ||
| .overlay( | ||
| GeometryReader { geometry in | ||
| Color.clear | ||
| .preference(key: ViewHeightKey.self, value: geometry.size.height) | ||
| } | ||
| ) | ||
| .onPreferenceChange(ViewHeightKey.self) { height in | ||
| withAnimation { | ||
| textFieldHeight = height | ||
| } | ||
| } | ||
| } | ||
| .frame(maxHeight: min(textFieldHeight, 130)) | ||
| } |
There was a problem hiding this comment.
The ScrollViewReader is created with a proxy parameter but it's never used. If there's no need to programmatically scroll to specific positions, you can remove the ScrollViewReader wrapper and just use ScrollView directly:
ScrollView {
TextField("Message", text: $message.animation(.easeOut(duration: 0.3)), axis: .vertical)
// ... rest of the code
}
.frame(maxHeight: min(textFieldHeight, 130))![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
- Remove unused ScrollViewReader wrapper, use ScrollView directly - Revert voiceIdentifier @AppStorage to empty string default (function calls not allowed as compile-time constants) The fallback to system default is already handled in getVoiceIdentifier()
- Define maxTextFieldHeight constant instead of magic number 130 - Apply duplicate removal and sorting to fallback voice list (previously only applied when language filter matched)
eulicesl
deleted the
claude/review-upstream-improvements-01Er6FSay4ot7btdZkakf5Nn
branch
PR fix: make macOS input field scrollable gluonfield/enchanted#131: Make macOS input field scrollable when text exceeds max height
PR Updates to Voice list in settings: removing duplicates, other languages gluonfield/enchanted#189: Improve voice list in settings
PR iOS 18 Dark Mode Icon #157 gluonfield/enchanted#186: Add iOS 18 dark mode icon support
Note
Makes macOS chat input scrollable with capped height, improves TTS voice selection/labeling with sane defaults, and adds iOS dark/tinted app icon variants (plus .gitignore).
TextFieldinScrollViewand addViewHeightKeyfor height updates (Enchanted/UI/macOS/Chat/Components/InputFields_macOS.swift).SpeechService.fetchVoices).SpeechService.getVoiceIdentifier,systemDefaultVoiceIdentifier).prettyNameand newAVSpeechSynthesisVoiceQuality.displayString(Enchanted/Extensions/AVSpeechSynthesisVoice+Extension.swift).Assets.xcassets/AppIcon.appiconset/Contents.json..gitignoreentry for.DS_Store.Written by Cursor Bugbot for commit bfe14e4. This will update automatically on new commits. Configure here.