Move hardcoded view strings to models and adopt playolaAlert#316
Open
briankeane wants to merge 8 commits into
Open
Move hardcoded view strings to models and adopt playolaAlert#316briankeane wants to merge 8 commits into
briankeane wants to merge 8 commits into
Conversation
playolaAlert's button builder renders an inert OK and drops dismissButton actions and secondary buttons. RecordPage (Settings button), AskQuestionPage and ListenerQuestionDetailPage (onDismiss) rely on those, so they keep the legacy alert until PlayolaAlert gains a single-button-with-action form.
Contributor
Greptile SummaryThis PR extracts ~95 hardcoded user-facing strings from 24 view files into computed properties on their corresponding
Confidence Score: 4/5Safe to merge with one follow-up: ListenerQuestionRow still contains nil-coalescing computed properties in the view that should be moved to the model. The vast majority of changes are mechanical string lifts with no behavioral impact. The one gap is in BroadcastersListenerQuestionPageView — ListenerQuestionRow.transcription and listenerName still perform nil-coalescing in the view, which is the same pattern RecordPage correctly fixed in this PR. Everything else (string extraction, static DateFormatter, playolaAlert adoption, RecordPage logic move) is clean and consistent with the project's architecture. PlayolaRadio/Views/Pages/BroadcastersListenerQuestionPage/BroadcastersListenerQuestionPageView.swift — ListenerQuestionRow still has conditional display logic in the view. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart LR
subgraph View["View (layout + rendering only)"]
T["Text(model.label)"]
B["Button(model.buttonTitle)"]
A[".playolaAlert($model.presentedAlert)"]
end
subgraph Model["@Observable Model (all content)"]
NL["var navigationTitle: String"]
BL["var buttonTitle: String"]
CH["func airtimeLabel(for:) -> String"]
ET["func expandToggleText(for:) -> String"]
PA["var presentedAlert: PlayolaAlert?"]
end
NL -->|string| T
BL -->|string| B
CH -->|string| T
ET -->|string| T
PA -->|binding| A
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart LR
subgraph View["View (layout + rendering only)"]
T["Text(model.label)"]
B["Button(model.buttonTitle)"]
A[".playolaAlert($model.presentedAlert)"]
end
subgraph Model["@Observable Model (all content)"]
NL["var navigationTitle: String"]
BL["var buttonTitle: String"]
CH["func airtimeLabel(for:) -> String"]
ET["func expandToggleText(for:) -> String"]
PA["var presentedAlert: PlayolaAlert?"]
end
NL -->|string| T
BL -->|string| B
CH -->|string| T
ET -->|string| T
PA -->|binding| A
Reviews (4): Last reviewed commit: "Move show more/less label selection into..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
- RecordPageView: collapse phase switch into model.currentButtonLabel so the view holds no conditional display logic. - BroadcastPageModel: hoist airtimeLabel's DateFormatter to a reused static property instead of allocating one per call.
…ring-and-alert-cleanup
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughUI strings now come from page models and reusable components instead of literals. Several screens also switched alert presentation to ChangesUI String Centralization and Alert Migration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Contributor
Author
|
@greptile review this |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
PlayolaRadio/Views/Pages/HomePage/UI Components/CircleSection.swift (1)
10-26: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMake
introMessagerequired instead of keeping fallback copy in the view.The new parameterization is good, but Line 10 still leaves the old user-facing string embedded in the component. That makes it easy for a future call site to skip injection and drift away from model-owned copy again.
Suggested change
- var introMessage: String = "Discover music through independent artist made radio stations." + let introMessage: String🤖 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 `@PlayolaRadio/Views/Pages/HomePage/UI` Components/CircleSection.swift around lines 10 - 26, Make CircleSection’s introMessage required instead of defaulting to embedded fallback copy. Update the CircleSection view definition so the caller must supply introMessage (alongside onIconTapped10Times), and remove the hardcoded user-facing string from the property declaration to keep the text source externalized and consistent with the rest of the UI.PlayolaRadio/Views/Pages/SongSearchPage/SongSearchPageView.swift (1)
81-93: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winMove the seed-row action branching into the model.
This button still decides in the view whether to call
onAddSongToLibraryoronRequestSong. Expose one model action for this tap and call only that from the view.As per path instructions, `**/*View.swift`: SwiftUI views should call model methods for all user actions and avoid logic in the view.♻️ Suggested direction
SongRequestResultRow( songRequest: songRequest, buttonText: model.songSeedActionButtonText, isProcessing: model.isProcessingAdd(for: songRequest) ) { - Task { - if model.isLibraryAddMode { - await model.onAddSongToLibrary(songRequest) - } else { - await model.onRequestSong(songRequest) - } - } + Task { await model.songSeedActionButtonTapped(songRequest) } }🤖 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 `@PlayolaRadio/Views/Pages/SongSearchPage/SongSearchPageView.swift` around lines 81 - 93, The SongSearchPageView still contains tap-time branching between onAddSongToLibrary and onRequestSong, so move that decision into the SongSearchPageViewModel by exposing a single action for the seed-row tap. Update the SongRequestResultRow callback to call only that model method, and have the model internally route to the correct behavior based on isLibraryAddMode while preserving the existing button text and processing state.Source: Coding guidelines
🤖 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
`@PlayolaRadio/Views/Pages/BroadcastersListenerQuestionPage/BroadcastersListenerQuestionPageView.swift`:
- Around line 155-159: Remove the hardcoded fallback text defaults from
ListenerQuestionRow so the view no longer owns any user-facing strings. Make the
text properties required inputs, and update the initializer/call sites to pass
model-owned values for answeredBadgeText, missingTranscriptionText,
unknownListenerText, showMoreText, and showLessText. Keep the view focused on
layout and presentation only, using the existing ListenerQuestionRow symbols to
locate and adjust the bindings.
In `@PlayolaRadio/Views/Pages/ContactPage/ContactPageModel.swift`:
- Around line 75-83: The ContactPageModel property block is missing the required
page-model organization and should be placed under the prescribed // MARK:
sections. Update ContactPageModel to restore the standard order by adding the
missing Initialization, User Actions, View Helpers, and Private Helpers sections
and moving the label/title properties into the correct Properties section while
keeping the existing Dependencies and Shared State structure intact.
In
`@PlayolaRadio/Views/Pages/ConversationListPage/ConversationListPageModel.swift`:
- Around line 22-24: The ConversationListPageModel file is missing the required
section organization, so add the prescribed // MARK: sections in the expected
order and place the existing display-string properties under the proper
Properties section. Update ConversationListPageModel to follow the repository’s
page model layout: Dependencies, Shared State, Initialization, Properties, User
Actions, View Helpers, then Private Helpers, keeping navigationTitle and
emptyStateMessage within Properties.
In `@PlayolaRadio/Views/Pages/EditProfilePage/EditProfilePageModel.swift`:
- Around line 28-35: The EditProfilePageModel is adding display-text properties
without the required section organization. Move these properties under the
existing prescribed `// MARK: - Properties` area and ensure the file follows the
standard order for page models: Dependencies, Shared State, Initialization,
Properties, User Actions, View Helpers, then Private Helpers. Use the
EditProfilePageModel symbol to keep the structure consistent as you touch this
model.
In `@PlayolaRadio/Views/Pages/HomePage/UI` Components/HomePageStationList.swift:
- Around line 85-93: The HomePageStationList component still exposes a hardcoded
fallback title via its title property, which lets call sites bypass model-owned
copy; remove the default value and require title to be passed in explicitly.
Update the HomePageStationList view API so title is a required dependency
alongside stations, liveStatusForStation, hasUpcomingGiveawayForStation, and
onRadioStationSelected, keeping the header text supplied by the parent/model.
In
`@PlayolaRadio/Views/Pages/NotificationsSettingsPage/NotificationsSettingsPageModel.swift`:
- Around line 64-69: The new display-text properties in
NotificationsSettingsPageModel are not placed under the required page-model
section structure. Move these computed properties into the `// MARK: -
Properties` section and ensure the file follows the prescribed order for
`NotificationsSettingsPageModel` (Dependencies, Shared State, Initialization,
Properties, User Actions, View Helpers, Private Helpers) so the new
titles/subtitles are grouped correctly.
In `@PlayolaRadio/Views/Pages/RecordPage/RecordPageModel.swift`:
- Around line 59-84: The display helper properties in RecordPageModel need to be
moved into the standard page-model section layout. Reorder and rename the
surrounding MARK sections in RecordPageModel so the file follows the required
sequence: Dependencies, Shared State, Initialization, Properties, User Actions,
View Helpers, then Private Helpers. Keep the helper properties like
navigationTitle, doneButtonTitle, waveformPlaceholderText, recordingStatusText,
recordButtonLabel, stopButtonLabel, reRecordButtonLabel, currentButtonLabel,
discardButtonTitle, and useRecordingButtonTitle grouped under the View Helpers
section.
In `@PlayolaRadio/Views/Pages/RewardsPage/RewardsPageModel.swift`:
- Around line 75-86: The RewardsPageModel file has page-model sections out of
the required order, with the new display properties/helpers placed under the
wrong MARK grouping. Reorganize the surrounding section labels and members in
RewardsPageModel so it follows the prescribed order: Dependencies, Shared State,
Initialization, Properties, User Actions, View Helpers, then Private Helpers.
Make sure navigationTitle, yourRewardsTitle, yourRewardsSubtitle, and
prizeTierHoursToGoText are placed under the correct section for view
helpers/properties per the repository convention.
- Around line 81-84: The countdown text in
RewardsPageModel.prizeTierHoursToGoText(for:) always pluralizes “hours,” so
update the formatting to handle the singular case when hoursToGo is 1. Keep the
logic in prizeTierHoursToGoText(for:) and return “1 hour to go” for one hour,
while preserving the existing plural text for all other values.
In `@PlayolaRadio/Views/Pages/SeriesListPage/EpisodeRow/EpisodeRowModel.swift`:
- Around line 45-48: The page model is missing the required // MARK section
structure, so update EpisodeRowModel to follow the prescribed order:
Dependencies, Shared State, Initialization, Properties, User Actions, View
Helpers, then Private Helpers. Move the new display-facing helpers like
episodeTitle and originallyAiredText into the View Helpers section and keep the
file organized consistently with the other page models. Use the existing
EpisodeRowModel symbol to place the properties in the right section and align
the header layout before adding more helpers.
In `@PlayolaRadio/Views/Pages/SeriesListPage/SeriesCard/SeriesCardModel.swift`:
- Around line 28-39: The SeriesCardModel display properties were added without
the required page-model MARK organization, so reorganize this type to match the
repo’s standard section order. In SeriesCardModel, move the new computed UI
fields into the proper // MARK sections and ensure the file follows
Dependencies, Shared State, Initialization, Properties, User Actions, View
Helpers, then Private Helpers, with these display-only getters grouped under
View Helpers rather than left as unsectioned helpers.
In `@PlayolaRadio/Views/Pages/SeriesListPage/SeriesListPageModel.swift`:
- Around line 28-29: The SeriesListPageModel file still uses the old
State/Init/Actions organization, so restore it to the required page-model layout
while keeping navigationTitle in the correct section. Reorder the type into the
prescribed // MARK: sections in this order: Dependencies, Shared State,
Initialization, Properties, User Actions, View Helpers, then Private Helpers,
and place navigationTitle under View Helpers since it is only a display helper.
Use the SeriesListPageModel symbol to update the structure without changing
behavior.
In `@PlayolaRadio/Views/Pages/SongSearchPage/SongSearchPageModel.swift`:
- Around line 67-74: The display helper properties in SongSearchPageModel are in
the wrong place and the page-model is missing the required // MARK: section
structure. Move these view-facing strings, including the conditional
songSeedActionButtonText, under the View Helpers section in SongSearchPageModel,
and ensure the file follows the prescribed section order: Dependencies, Shared
State, Initialization, Properties, User Actions, View Helpers, then Private
Helpers.
In `@PlayolaRadio/Views/Pages/SongSearchPage/SongSearchPageView.swift`:
- Around line 142-145: SongSearchResultRow is still using a hardcoded fallback
for buttonText, which allows user-facing copy to come from the view instead of
the caller/model. Make buttonText a required input on SongSearchResultRow and
update any call sites in SongSearchPageView or related builders to pass the
label explicitly, removing the default "SELECT" so the view no longer supplies
its own copy.
In
`@PlayolaRadio/Views/Pages/StationSuggestionPage/StationSuggestionPageModel.swift`:
- Around line 127-129: The vote label in voteCountLabel is always using the
plural suffix, which produces incorrect text for a single vote. Update
voteCountLabel to keep using voteCountText but choose the suffix based on
suggestion.voteCount so it renders “vote” when the count is 1 and “votes”
otherwise.
In `@PlayolaRadio/Views/Pages/SupportPage/SupportPageModel.swift`:
- Around line 36-38: The SupportPageModel is missing the required page-model
organization blocks, so add the prescribed // MARK: sections in the expected
order and place the new display-text properties under the Properties section.
Use the SupportPageModel type as the anchor, and ensure the file is structured
with Dependencies, Shared State, Initialization, Properties, User Actions, View
Helpers, then Private Helpers so it matches the coding guidelines.
---
Outside diff comments:
In `@PlayolaRadio/Views/Pages/HomePage/UI` Components/CircleSection.swift:
- Around line 10-26: Make CircleSection’s introMessage required instead of
defaulting to embedded fallback copy. Update the CircleSection view definition
so the caller must supply introMessage (alongside onIconTapped10Times), and
remove the hardcoded user-facing string from the property declaration to keep
the text source externalized and consistent with the rest of the UI.
In `@PlayolaRadio/Views/Pages/SongSearchPage/SongSearchPageView.swift`:
- Around line 81-93: The SongSearchPageView still contains tap-time branching
between onAddSongToLibrary and onRequestSong, so move that decision into the
SongSearchPageViewModel by exposing a single action for the seed-row tap. Update
the SongRequestResultRow callback to call only that model method, and have the
model internally route to the correct behavior based on isLibraryAddMode while
preserving the existing button text and processing state.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 204b8031-8599-484c-9bcc-04cd7b801f45
📒 Files selected for processing (44)
PlayolaRadio/Views/Pages/AskQuestionPage/AskQuestionPageModel.swiftPlayolaRadio/Views/Pages/AskQuestionPage/AskQuestionPageView.swiftPlayolaRadio/Views/Pages/BroadcastPage/BroadcastPageModel.swiftPlayolaRadio/Views/Pages/BroadcastPage/BroadcastPageView.swiftPlayolaRadio/Views/Pages/BroadcastersListenerQuestionPage/BroadcastersListenerQuestionPageModel.swiftPlayolaRadio/Views/Pages/BroadcastersListenerQuestionPage/BroadcastersListenerQuestionPageView.swiftPlayolaRadio/Views/Pages/ContactPage/ContactPageModel.swiftPlayolaRadio/Views/Pages/ContactPage/ContactPageView.swiftPlayolaRadio/Views/Pages/ConversationListPage/ConversationListPageModel.swiftPlayolaRadio/Views/Pages/ConversationListPage/ConversationListPageView.swiftPlayolaRadio/Views/Pages/EditProfilePage/EditProfilePageModel.swiftPlayolaRadio/Views/Pages/EditProfilePage/EditProfilePageView.swiftPlayolaRadio/Views/Pages/FeedbackSheet/FeedbackSheetView.swiftPlayolaRadio/Views/Pages/HomePage/HomePageModel.swiftPlayolaRadio/Views/Pages/HomePage/HomePageView.swiftPlayolaRadio/Views/Pages/HomePage/UI Components/CircleSection.swiftPlayolaRadio/Views/Pages/HomePage/UI Components/HomePageStationList.swiftPlayolaRadio/Views/Pages/ListenerQuestionDetailPage/ListenerQuestionDetailPageModel.swiftPlayolaRadio/Views/Pages/ListenerQuestionDetailPage/ListenerQuestionDetailPageView.swiftPlayolaRadio/Views/Pages/MainContainer/MainContainer.swiftPlayolaRadio/Views/Pages/MainContainer/MainContainerModel.swiftPlayolaRadio/Views/Pages/NotificationsSettingsPage/NotificationsSettingsPageModel.swiftPlayolaRadio/Views/Pages/NotificationsSettingsPage/NotificationsSettingsPageView.swiftPlayolaRadio/Views/Pages/RecordPage/RecordPageModel.swiftPlayolaRadio/Views/Pages/RecordPage/RecordPageView.swiftPlayolaRadio/Views/Pages/RewardsPage/PrizeTierRow.swiftPlayolaRadio/Views/Pages/RewardsPage/RewardsPageModel.swiftPlayolaRadio/Views/Pages/RewardsPage/RewardsPageView.swiftPlayolaRadio/Views/Pages/SeriesListPage/EpisodeRow/EpisodeRow.swiftPlayolaRadio/Views/Pages/SeriesListPage/EpisodeRow/EpisodeRowModel.swiftPlayolaRadio/Views/Pages/SeriesListPage/SeriesCard/SeriesCard.swiftPlayolaRadio/Views/Pages/SeriesListPage/SeriesCard/SeriesCardModel.swiftPlayolaRadio/Views/Pages/SeriesListPage/SeriesListPage.swiftPlayolaRadio/Views/Pages/SeriesListPage/SeriesListPageModel.swiftPlayolaRadio/Views/Pages/SignInPage/SignInPageModel.swiftPlayolaRadio/Views/Pages/SignInPage/SignInPageView.swiftPlayolaRadio/Views/Pages/SongSearchPage/SongSearchPageModel.swiftPlayolaRadio/Views/Pages/SongSearchPage/SongSearchPageView.swiftPlayolaRadio/Views/Pages/StationSuggestionPage/StationSuggestionPageModel.swiftPlayolaRadio/Views/Pages/StationSuggestionPage/StationSuggestionPageView.swiftPlayolaRadio/Views/Pages/SupportPage/SupportPageModel.swiftPlayolaRadio/Views/Pages/SupportPage/SupportPageView.swiftPlayolaRadio/Views/Reusable Components/ListeningTimeTile/ListeningTimeTile.swiftPlayolaRadio/Views/Reusable Components/ListeningTimeTile/ListeningTimeTileModel.swift
- Pluralize '1 hour to go' (RewardsPageModel) and '1 vote' (StationSuggestionPageModel). - Drop hardcoded default values from presentational subview params (HomePageStationList, HomeIntroSection, SongSearchResultRow, SongRequestResultRow, ListenerQuestionRow) so display text comes only from the model; previews/call sites pass explicit values.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
PlayolaRadio/Views/Pages/BroadcastersListenerQuestionPage/BroadcastersListenerQuestionPageView.swift (1)
169-173: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winMove the fallback display selection into the page model.
ListenerQuestionRowstill decides which user-facing text to render when transcription or listener name is missing. Pass display-ready strings from the model instead so this view stays presentation-only. As per coding guidelines,**/*View.swift: SwiftUI views should handle visuals only: layout, spacing, colors, and fonts; bind to model properties for all content; call model methods for all user actions; and avoid hardcoded strings or logic in the view.🤖 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 `@PlayolaRadio/Views/Pages/BroadcastersListenerQuestionPage/BroadcastersListenerQuestionPageView.swift` around lines 169 - 173, `ListenerQuestionRow` is still choosing fallback text for missing transcription and listener name instead of receiving display-ready content from the page model. Move that selection logic out of `BroadcastersListenerQuestionPageView` into the corresponding model/view model, then have the view bind directly to model-provided strings for transcription and listener display text. Keep `ListenerQuestionRow` presentation-only by removing the `question.transcription ?? missingTranscriptionText` and `question.listener?.fullName ?? unknownListenerText` logic from the view.Source: Coding guidelines
🤖 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.
Outside diff comments:
In
`@PlayolaRadio/Views/Pages/BroadcastersListenerQuestionPage/BroadcastersListenerQuestionPageView.swift`:
- Around line 169-173: `ListenerQuestionRow` is still choosing fallback text for
missing transcription and listener name instead of receiving display-ready
content from the page model. Move that selection logic out of
`BroadcastersListenerQuestionPageView` into the corresponding model/view model,
then have the view bind directly to model-provided strings for transcription and
listener display text. Keep `ListenerQuestionRow` presentation-only by removing
the `question.transcription ?? missingTranscriptionText` and
`question.listener?.fullName ?? unknownListenerText` logic from the view.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 07630590-f414-41db-9fbe-add4ccfe805d
📒 Files selected for processing (6)
PlayolaRadio/Views/Pages/BroadcastersListenerQuestionPage/BroadcastersListenerQuestionPageView.swiftPlayolaRadio/Views/Pages/HomePage/UI Components/CircleSection.swiftPlayolaRadio/Views/Pages/HomePage/UI Components/HomePageStationList.swiftPlayolaRadio/Views/Pages/RewardsPage/RewardsPageModel.swiftPlayolaRadio/Views/Pages/SongSearchPage/SongSearchPageView.swiftPlayolaRadio/Views/Pages/StationSuggestionPage/StationSuggestionPageModel.swift
Move the string-extraction display-text computed properties/helpers in 11 page models from the Properties area into a // MARK: - View Helpers section placed after User Actions, per the project's model section ordering. Position/MARK only, no logic or string changes.
Contributor
Author
|
@greptile review this |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
PlayolaRadio/Views/Pages/SeriesListPage/EpisodeRow/EpisodeRowModel.swift (1)
49-49: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAvoid rendering a dangling
First Airedlabel.When
airing.episode?.createdAtis missing,originallyAiredDateTextreturns"", so this property still emitsFirst Aired.Proposed fix
- var originallyAiredText: String { "First Aired \(originallyAiredDateText)" } + var originallyAiredText: String { + guard !originallyAiredDateText.isEmpty else { return "" } + return "First Aired \(originallyAiredDateText)" + }🤖 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 `@PlayolaRadio/Views/Pages/SeriesListPage/EpisodeRow/EpisodeRowModel.swift` at line 49, The originallyAiredText computed property in EpisodeRowModel should avoid showing a bare “First Aired” label when originallyAiredDateText is empty. Update the formatting logic so it only returns the full label when airing.episode?.createdAt exists, and otherwise returns an empty string or omits the label entirely. Keep the change localized to the EpisodeRowModel properties that build originallyAiredText/originallyAiredDateText.
🤖 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.
Outside diff comments:
In `@PlayolaRadio/Views/Pages/SeriesListPage/EpisodeRow/EpisodeRowModel.swift`:
- Line 49: The originallyAiredText computed property in EpisodeRowModel should
avoid showing a bare “First Aired” label when originallyAiredDateText is empty.
Update the formatting logic so it only returns the full label when
airing.episode?.createdAt exists, and otherwise returns an empty string or omits
the label entirely. Keep the change localized to the EpisodeRowModel properties
that build originallyAiredText/originallyAiredDateText.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 2c123941-28a4-463e-bb20-e01e7dddccd5
📒 Files selected for processing (11)
PlayolaRadio/Views/Pages/ContactPage/ContactPageModel.swiftPlayolaRadio/Views/Pages/ConversationListPage/ConversationListPageModel.swiftPlayolaRadio/Views/Pages/EditProfilePage/EditProfilePageModel.swiftPlayolaRadio/Views/Pages/NotificationsSettingsPage/NotificationsSettingsPageModel.swiftPlayolaRadio/Views/Pages/RecordPage/RecordPageModel.swiftPlayolaRadio/Views/Pages/RewardsPage/RewardsPageModel.swiftPlayolaRadio/Views/Pages/SeriesListPage/EpisodeRow/EpisodeRowModel.swiftPlayolaRadio/Views/Pages/SeriesListPage/SeriesCard/SeriesCardModel.swiftPlayolaRadio/Views/Pages/SeriesListPage/SeriesListPageModel.swiftPlayolaRadio/Views/Pages/SongSearchPage/SongSearchPageModel.swiftPlayolaRadio/Views/Pages/SupportPage/SupportPageModel.swift
ListenerQuestionRow chose between showLessText/showMoreText with an in-view ternary. Add expandToggleText(for:) on the model and pass the resolved string, matching the RecordPage/AskQuestionPage pattern so the view holds no copy- selection logic. The row keeps isExpanded for the chevron and line limit.
Contributor
Author
|
@greptile review this |
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
1 participant
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.
Moves ~95 hardcoded user-facing strings (nav titles, labels, button text, empty-state copy, placeholders) out of 24 page views into computed properties on their
@Observablemodels, so the model owns all display text per the MV architecture. Also swaps raw.alert(item: $model.presentedAlert) { $0.alert }for the project's.playolaAlert($model.presentedAlert)modifier wherever it's behavior-preserving. RecordPage, AskQuestionPage, and ListenerQuestionDetailPage keep the legacy.alertbecause their alerts carry adismissButtonaction or a secondary button, which.playolaAlert's button builder does not yet support (it would render an inert OK and drop the action). PlayerPage is intentionally left for a follow-up PR (it also needs aUIScreen.mainlayout migration). Behavior is unchanged: full test suite passes and an independent Codex review confirmed no regressions.Summary by CodeRabbit