Add General / Permissions / About panes to redesigned Settings#364
Merged
Conversation
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.
Re-opened after base branch was deleted by the PR #356 merge. Same content as the closed #357, rebased onto current main (incorporates the tooltip-removal PR #362 — every
.cotabbyHelp(...)chain is gone) plus the Greptile fixes for scene-active refresh and safer URL handling.Validation
Risk / rollout
Stacked: depends on PR #356 being on main, which it now is. Next in the stack will rebase onto this.
Greptile Summary
Promotes three Settings panes — General, Permissions, and About — from the
PlaceholderPaneViewstub to their real implementations, and adds an Acknowledgements modal sheet accessible from the About pane.SettingsPaneScaffoldlayout, preserving all bindings and setters unchanged.onAppearfor initial load andscenePhase .activefor return-from-System-Settings).appVersionTextcorrectly handles all four combinations ofCFBundleShortVersionStringandCFBundleVersion.Confidence Score: 4/5
Safe to merge; changes are additive UI panes with no shared state mutations or new persistence logic.
All three panes are straightforward SwiftUI views that delegate to pre-existing, already-tested model objects. The only nit is that
AcknowledgementEntrystores its URL as a rawString, which could silently hide a link icon if a future entry contains a typo — no present misbehaviour, just a latent maintenance trap.No files require special attention;
AcknowledgementsView.swifthas the minor URL-typing concern noted in the inline comment.Important Files Changed
urlstored asStringrather thanURLmeans a future typo silently hides the link iconFlowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD SCV[SettingsContainerView] -->|selection == .general| GPV[GeneralPaneView] SCV -->|selection == .permissions| PPV[PermissionsPaneView] SCV -->|selection == .about| APV[AboutPaneView] SCV -->|other cases| PH[PlaceholderPaneView] GPV -->|reads/writes| SSM[SuggestionSettingsModel] GPV -->|callback| OWC[onShowWelcome] PPV -->|observes| PM[PermissionManager] PPV -->|onAppear| PM PPV -->|scenePhase .active| PM APV -->|checkForUpdates| AUM[AppUpdateManager] APV -->|isShowingAcknowledgements| ACV[AcknowledgementsView sheet] ACV -->|static entries| ACE[AcknowledgementEntry list]Comments Outside Diff (1)
Cotabby/UI/Settings/Panes/AcknowledgementsView.swift, line 311-314 (link)urlstored asStringsilently drops link icons on typoAcknowledgementEntry.urlis a rawString. InAcknowledgementRowtheif let url = URL(string: entry.url)guard silently omits the external-link icon if the string isn't a valid URL. All four current entries are valid, but a future maintainer adding a typo'd URL (e.g. a missing scheme) would get a link-less row with no compile-time or runtime warning. Storing the value asURLinstead would catch mistakes at the call site and remove the defensiveif letin the row view.Reviews (1): Last reviewed commit: "Address Greptile feedback: scene-active ..." | Re-trigger Greptile