Refine menu bar ticker settings#5
Open
1254455745 wants to merge 1 commit into
Open
Conversation
|
Review mode finished without calling create_pull_request_review after 3 retry attempts
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (11)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthrough本PR为Ticker菜单栏显示内容实现可配置化：新增持久化配置key与业务模型属性，通过协议要求与视图实现支持动态图标显示，并在内容渲染与用户设置界面中完整集成这些选项。 ChangesTicker显示项配置与视图适配
Sequence Diagram(s)无。本PR采用属性驱动的声明式设计，不涉及多组件的顺序交互流程。 Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@PanBar/Infrastructure/TickerPreferences.swift`:
- Around line 170-173: The migration maps storedDisplayMode == .scrollNoCode to
.scroll but only migrates/writes back when migratesMinimal is true; update the
migration in the initializer/where displayMode is set so that when
storedDisplayMode == .scrollNoCode you also call
repo.set(SettingsRepository.Keys.tickerDisplayMode,
TickerDisplayMode.scroll.rawValue) (similar to the migratesMinimal branch) to
persist the migrated value and prevent showQuoteCode from being repeatedly
reset; make the same persistence change in the corresponding mirror block around
the displayMode handling at the 177-181 region.
In `@PanBar/Settings/Panes/TickerPane.swift`:
- Around line 75-82: The UI currently shows the width Stepper for both .compact
and .minimal in TickerPane.swift even though StatusItemController.swift
explicitly ignores width in the .minimal branch (preferredTotalWidth = nil), so
hide the width control for .minimal: change the switch handling of .compact and
.minimal (or add an explicit if) so that Toggle(L(...), isOn:
$prefs.compactAutoWidth) can remain, but the Stepper(value:
$prefs.compactMenuBarWidth, ...) is only rendered when the mode is .compact
(i.e., show Stepper only when current mode == .compact), preventing users from
editing an option that StatusItemController ignores.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 019b8f13-943e-4d75-890a-25cbacd3c633
📒 Files selected for processing (12)
PanBar/Data/Persistence/Repositories/SettingsRepository.swiftPanBar/Infrastructure/TickerPreferences.swiftPanBar/MenuBar/CarouselTickerView.swiftPanBar/MenuBar/CompactTickerView.swiftPanBar/MenuBar/MenuBarTickerView.swiftPanBar/MenuBar/MinimalTickerView.swiftPanBar/MenuBar/StatusItemController.swiftPanBar/MenuBar/TickerRenderer.swiftPanBar/MenuBar/TickerView.swiftPanBar/Popover/PopoverController.swiftPanBar/Resources/Localizable.xcstringsPanBar/Settings/Panes/TickerPane.swift
Comment on lines
+170
to
+173
| self.displayMode = storedDisplayMode == .scrollNoCode ? .scroll : (migratesMinimal ? .compact : storedDisplayMode) | ||
| if migratesMinimal { | ||
| try? repo.set(SettingsRepository.Keys.tickerDisplayMode, TickerDisplayMode.compact.rawValue) | ||
| } |
There was a problem hiding this comment.
scrollNoCode 迁移未落库，showQuoteCode 会被反复重置
Line 170 把旧值映射成 .scroll，但只有 .minimal 分支在 Line 171-173 做了写回。当前实现会让 storedDisplayMode 长期停留在 .scrollNoCode，并在 Line 177 每次启动都强制 showQuoteCode = false，覆盖用户后续设置。建议在 scrollNoCode 分支同样做一次迁移写回。
🔧 建议修改
- self.displayMode = storedDisplayMode == .scrollNoCode ? .scroll : (migratesMinimal ? .compact : storedDisplayMode)
+ let migratesScrollNoCode = storedDisplayMode == .scrollNoCode
+ self.displayMode = migratesScrollNoCode ? .scroll : (migratesMinimal ? .compact : storedDisplayMode)
+ if migratesScrollNoCode {
+ try? repo.set(SettingsRepository.Keys.tickerDisplayMode, TickerDisplayMode.scroll.rawValue)
+ }
if migratesMinimal {
try? repo.set(SettingsRepository.Keys.tickerDisplayMode, TickerDisplayMode.compact.rawValue)
}Also applies to: 177-181
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@PanBar/Infrastructure/TickerPreferences.swift` around lines 170 - 173, The
migration maps storedDisplayMode == .scrollNoCode to .scroll but only
migrates/writes back when migratesMinimal is true; update the migration in the
initializer/where displayMode is set so that when storedDisplayMode ==
.scrollNoCode you also call repo.set(SettingsRepository.Keys.tickerDisplayMode,
TickerDisplayMode.scroll.rawValue) (similar to the migratesMinimal branch) to
persist the migrated value and prevent showQuoteCode from being repeatedly
reset; make the same persistence change in the corresponding mirror block around
the displayMode handling at the 177-181 region.
Comment on lines
+75
to
+82
| case .compact, .minimal: | ||
| Toggle(L("ticker.autoMenuBarWidth", comment: ""), isOn: $prefs.compactAutoWidth) | ||
| if !prefs.compactAutoWidth { | ||
| Stepper(value: $prefs.compactMenuBarWidth, in: 60...360, step: 20) { | ||
| Text(String(format: L("ticker.menuBarWidth", comment: ""), prefs.compactMenuBarWidth)) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
.minimal 下的宽度设置当前是无效控件
这里把 .compact 和 .minimal 绑定到同一组宽度控件，但下游 PanBar/MenuBar/StatusItemController.swift 在 .minimal 分支固定不应用宽度（preferredTotalWidth = nil）。这会让用户在 minimal 模式下改设置却看不到效果。
🔧 建议修改（先避免暴露无效控件）
- case .compact, .minimal:
+ case .compact:
Toggle(L("ticker.autoMenuBarWidth", comment: ""), isOn: $prefs.compactAutoWidth)
if !prefs.compactAutoWidth {
Stepper(value: $prefs.compactMenuBarWidth, in: 60...360, step: 20) {
Text(String(format: L("ticker.menuBarWidth", comment: ""), prefs.compactMenuBarWidth))
}
}
+ case .minimal:
+ EmptyView()
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| case .compact, .minimal: | |
| Toggle(L("ticker.autoMenuBarWidth", comment: ""), isOn: $prefs.compactAutoWidth) | |
| if !prefs.compactAutoWidth { | |
| Stepper(value: $prefs.compactMenuBarWidth, in: 60...360, step: 20) { | |
| Text(String(format: L("ticker.menuBarWidth", comment: ""), prefs.compactMenuBarWidth)) | |
| } | |
| } | |
| } | |
| case .compact: | |
| Toggle(L("ticker.autoMenuBarWidth", comment: ""), isOn: $prefs.compactAutoWidth) | |
| if !prefs.compactAutoWidth { | |
| Stepper(value: $prefs.compactMenuBarWidth, in: 60...360, step: 20) { | |
| Text(String(format: L("ticker.menuBarWidth", comment: ""), prefs.compactMenuBarWidth)) | |
| } | |
| } | |
| case .minimal: | |
| EmptyView() | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@PanBar/Settings/Panes/TickerPane.swift` around lines 75 - 82, The UI
currently shows the width Stepper for both .compact and .minimal in
TickerPane.swift even though StatusItemController.swift explicitly ignores width
in the .minimal branch (preferredTotalWidth = nil), so hide the width control
for .minimal: change the switch handling of .compact and .minimal (or add an
explicit if) so that Toggle(L(...), isOn: $prefs.compactAutoWidth) can remain,
but the Stepper(value: $prefs.compactMenuBarWidth, ...) is only rendered when
the mode is .compact (i.e., show Stepper only when current mode == .compact),
preventing users from editing an option that StatusItemController ignores.
Owner
|
@1254455745 原来的极简风格不要删除 |
Author
我看那个极简和固定区别不大，就多安排了几个按钮，可以手动调整出来
你看看需不需要吧
|
Owner
|
@1254455745 可以拆一下 比如那个自动宽度 我认为还是可以的 几个几个开关也是 你拆的越细越好 |
1254455745
force-pushed
the
split/menu-ticker-settings
branch
from
e3f1083 to
97d12bb
Compare
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes — the single-commit incremental delta adds three menu bar content display toggles (app icon, stock code, stock name) with migration from the legacy scrollNoCode display mode to the new showQuoteCode-based model.
- Add
showAppIcon,showQuoteCode,showQuoteNametoggles — newSettingsRepositorykeys,TickerPreferencesproperties with auto-persist, and corresponding UI in the Ticker settings pane - Deprecate
scrollNoCodedisplay mode — migration maps the old.scrollNoCodecase to.scrollon init, using thedisplayModedidSetobserver to persist the migrated value; initialshowQuoteCodedefaults tofalsefor users coming from.scrollNoCode - Add
showsIcontoMenuBarTickerViewprotocol — all four ticker view types (TickerView,CarouselTickerView,CompactTickerView,MinimalTickerView) adopt it, andStatusItemController.applyPrefs()sets it fromprefs.showAppIcon - Add
showsQuoteCode/showsQuoteNametoTickerRenderer— conditional rendering of stock code and name in the quote-attributed string - Conditional settings UI —
showsQuoteContentControlscomputed property guards the stock code/name toggles, showing them only for.scroll/.carouselmodes - Update localization strings — English and Simplified Chinese entries for the three new toggles and the "Display Content" section header
DeepSeek Flash ｜ 𝕏
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
2 participants
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.
Summary by CodeRabbit
发行说明