Conversation
| CURRENT_PROJECT_VERSION = 110; | ||
| DEVELOPMENT_TEAM = 9Z68336878; | ||
| GENERATE_INFOPLIST_FILE = YES; | ||
| MACOSX_DEPLOYMENT_TARGET = 26.0; |
There was a problem hiding this comment.
Коротко: по сути это механическое повышение CURRENT_PROJECT_VERSION с 109 до 110 во всех конфигурациях/таргетах. Работать будет, но есть несколько мест, где сейчас заложены риски и лишний шум в PR.
Рекомендации:
-
Централизуйте версионирование, чтобы не плодить десятки правок в pbxproj при каждом инкременте:
- Вынесите CURRENT_PROJECT_VERSION (и MARKETING_VERSION) на уровень проекта (Project, All Configurations) или в общий .xcconfig, и удалите переопределения на уровне таргетов.
- Добавьте VERSIONING_SYSTEM = apple-generic (если планируете использовать agvtool).
- В CI используйте agvtool new-version -all или fastlane increment_build_number, чтобы инкремент происходил автоматически и не требовал PR.
-
Приведите к единому подходу работу с Info.plist:
- Сейчас у ряда таргетов одновременно стоят GENERATE_INFOPLIST_FILE = YES и INFOPLIST_FILE = ... Это источник расхождений и трудноотлавливаемых багов. Выберите один вариант:
- Либо полностью перейти на сгенерированный Info.plist и убрать INFOPLIST_FILE.
- Либо оставить кастомные Info.plist и поставить GENERATE_INFOPLIST_FILE = NO.
- Если остаются кастомные Info.plist, убедитесь, что CFBundleVersion =
$(CURRENT_PROJECT_VERSION) (и CFBundleShortVersionString = $ (MARKETING_VERSION)), чтобы инкремент действительно попадал в бандлы расширений/виджетов.
- Сейчас у ряда таргетов одновременно стоят GENERATE_INFOPLIST_FILE = YES и INFOPLIST_FILE = ... Это источник расхождений и трудноотлавливаемых багов. Выберите один вариант:
-
Согласованность версий между контейнером и расширениями/виджетами:
- Хотя формально не обязательно, лучше держать одинаковые CFBundleVersion/CFBundleShortVersionString у приложения, виджетов и watch-расширений — это снижает риск предупреждений/отклонений в App Store Connect и упрощает поддержку.
-
Проверьте deployment targets:
- IPHONEOS_DEPLOYMENT_TARGET = 18.1 — это резко поднимает минимальную версию iOS. Если это не намеренно, снизьте (и сделайте единым местом правды, аналогично версиям).
- MACOSX_DEPLOYMENT_TARGET = 26.0 выглядит неправдоподобно для macOS. Уточните: для macOS актуальны 14.x/15.x; для Catalyst свои настройки. Это может быть ошибка, которая приведет к предупреждениям/сборочным проблемам.
-
DEVELOPMENT_TEAM:
- Убедитесь, что оно задано единообразно (лучше на уровне проекта) и не переопределяется лишний раз на таргетах без необходимости.
-
Сократите шум в тестовых таргетах:
- Тестовым бандлам не обязательно явно прописывать CURRENT_PROJECT_VERSION. Пусть наследуют — меньше правок в PR.
Итого: текущая правка допустима, но лучше упразднить дубли, выровнять политику Info.plist и deployment target’ов и автоматизировать инкремент сборочного номера. Это уменьшит вероятность расхождений и избавит от «инкрементных» PR.
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Ниже — только замечания по проблемным/сомнительным местам и предложениям улучшения.
Структура/дублирование
- Дублирование разметки строк (HStack с кнопками) в ветках OutlineGroup/else. Вынесите строку задачи в отдельный сабвью (например, FocusTaskRow(task:)), туда же перенесите логику запуска/остановки таймера. Это сократит ошибки и облегчит поддержку.
- Условие для OutlineGroup: сейчас используете visibleSubtasks != nil. Это меняет поведение по сравнению с hasSubtasks() и приведёт к тому, что для задач с пустым [], но не nil, вы строите OutlineGroup (лишняя вложенность/отступы). Правильнее проверять на непустоту: if let subs = task.visibleSubtasks, !subs.isEmpty { ... } или вообще всегда использовать OutlineGroup и не ветвиться — OutlineGroup корректно рендерит листья без детей.
- Рассмотрите использование одного OutlineGroup для всех корневых задач без ForEach-обёртки:
OutlineGroup(sortedTasks, id: .id, children: .visibleSubtasks) { task in Row(task) }
Это упростит дерево и идентичности.
Идентичности/selection
- id: .self в ForEach и OutlineGroup — потенциально опасно, если Hashable/Equatable зависят от изменяемых полей. Используйте стабильный идентификатор (например, task.id).
- .tag(...) для selection List лучше вешать на корневой вид строки (HStack), а не на вложенный TaskRowView — так надёжнее для работы выбора.
Сепараторы списка
- .listRowSeparator(.hidden) навешан местами на вложенные вью (TaskRowView), что не гарантирует эффекта. Применяйте к корню строки (HStack) или отключите глобально (если дизайн позволяет), либо последовательно ко всем строкам. Сейчас местами модификатор продублирован/не там где нужно.
Логика таймера/фокуса
- Дублирование и порядок действий:
- В стоп-кнопке: сначала reset(), потом, при mode == .pause || .longbreak, вызывается skip(). После reset мода уже может измениться, и skip() станет бессмысленным. Определите чёткую последовательность в одном месте (например, stopFocus()).
- В play-кнопке: одинаковые блоки start()/resume() и установка viewMode. Вынесите в startFocus(for:).
- Что если уже есть активный focusTask.task и пользователь нажимает play на другой задаче? Сейчас предыдущая не останавливается явно. Заложите переключение: stopFocus() предыдущей и startFocus(for:) новой — атомарно.
- viewMode = 1 — «магическое число». Используйте enum с RawRepresentable/CaseIterable и храните Binding.
Состояния/пустой список
- if tasksTodayActive.count > 0 — используйте isEmpty. При переходе к пустому списку очистите selection/focus, иначе останется несогласованное состояние (selectedTask указывает на исчезнувший элемент).
- Плейсхолдер при пустом списке лучше рендерить как overlay/ContentUnavailableView (iOS 17+) поверх пустого List, чтобы сохранить единообразный layout и жесты списка.
Сортировка/Query
- Вы каждый раз сортируете в body: tasksTodayActive.sorted(...). Перенесите сортировки в @query (sortDescriptors), либо вычисляйте один раз в computed-свойстве с memoization. Это снизит работу на каждое перерисовывание.
Доступность/идентификаторы
- .accessibility(identifier:) формируется из имени задачи. Имена не гарантируют уникальность/стабильность. Лучше использовать task.id.
- Добавьте .accessibilityLabel и .accessibilityHint для кнопок play/stop, чтобы не полагаться только на иконки. Например: accessibilityLabel("Запустить «(task.name)»").
Прочее
- Имена: maintask → mainTask (читабельность). Локализация "No tasks for today" — используйте локализуемую строку/ключ.
- Кнопки в строке списка: установите .buttonStyle(.plain), чтобы не конфликтовать с выделением строк и не ломать внешний вид List.
- С анимациями переключения фокуса/таймера будет приятнее: withAnimation { focusTask.task = ... }.
Рефакторинг-скетч (упрощённо, чтобы показать направления):
-
Вынести Row:
struct FocusRow: View {
let task: Todo
@binding var viewMode: FocusMode
@EnvironmentObject var timer: TimerModel
@EnvironmentObject var focusTask: FocusModelvar body: some View {
HStack {
TaskRowView(task: task)
Spacer(minLength: 8)
Button(action: toggle) { Image(systemName: isFocused ? "stop.fill" : "play.fill") }
.buttonStyle(.plain)
.accessibilityIdentifier("(task.id.uuidString)-PlayButton")
}
.tag(task.id)
.listRowSeparator(.hidden)
}private var isFocused: Bool { focusTask.task?.id == task.id }
private func toggle() {
if isFocused { stopFocus() } else { startFocus(for: task) }
}private func startFocus(for task: Todo) {
// единый порядок действий
}private func stopFocus() {
// единый порядок действий
}
} -
И заменить в основном виде на один OutlineGroup c id: .id и children: .visibleSubtasks (или разрулить условие isEmpty у детей).
Эти шаги уберут дубли, стабилизируют идентичности, улучшат доступность и сделают логику таймера предсказуемой.
| if task.visibleSubtasks != nil { | ||
| OutlineGroup([task], | ||
| id: \.self, | ||
| children: \.visibleSubtasks) { maintask in |
There was a problem hiding this comment.
Ниже — только по делу, по изменённой строке и ближайшему контексту.
-
Семантика условия. Заменив hasSubtasks() на visibleSubtasks != nil, вы полагаетесь на то, что visibleSubtasks возвращает nil при отсутствии детей. Если свойство возвращает пустой массив (не nil), вы получите “группу без детей” — разный UX на платформах: может появиться раскрывающийся маркер без фактических элементов. Надёжнее:
- Либо обеспечить, чтобы visibleSubtasks возвращало nil именно при пустоте.
- Либо проверять явно: if task.visibleSubtasks?.isEmpty == false { ... }.
-
Двойные вычисления. Вы вычисляете visibleSubtasks минимум дважды: в if и при построении OutlineGroup (через keyPath). Если visibleSubtasks — дорогая вычислимое свойство, это излишняя нагрузка. Снизить:
- Сохранить локально: let children = task.visibleSubtasks; if let children, !children.isEmpty { OutlineGroup(..., children: .visibleSubtasks) { ... } }.
- Или в модели ввести hasVisibleSubtasks: Bool { visibleSubtasks?.isEmpty == false }.
-
Идентификаторы. id: .self как для ForEach, так и для OutlineGroup — риск нестабильной идентичности и конфликтов при диффинге, особенно если Hashable зависит от изменяемых свойств (например, статуса/содержимого). Лучше использовать стабильный уникальный идентификатор:
- ForEach(..., id: .id)
- OutlineGroup(..., id: .id)
-
Консистентность сортировки. Родительский список отсортирован TasksQuery.sortingWithCompleted, а visibleSubtasks, вероятно, нет. Несогласованная сортировка между уровнями дерева даёт “прыжки” при обновлениях. Рекомендация: сортировать в visibleSubtasks тем же компаратором (и возвращать nil, если после фильтрации пусто).
-
Упростить ветвление. Если для задач без детей у вас есть отдельный рендеринг, оставляйте if. Если нет — можно вообще убрать проверку и всегда рендерить OutlineGroup: он корректно обрабатывает nil у детей (листовые узлы). Но этот вариант потребует строго соблюдать правило “nil — когда детей нет”, чтобы не было “пустых групп”.
Итоговое безопасное правило:
- visibleSubtasks всегда nil при отсутствии видимых детей, отсортирован не пустой массив при наличии.
- Проверка на наличие детей: task.visibleSubtasks?.isEmpty == false.
- Везде использовать стабильные id: .id.
| if task.visibleSubtasks != nil { | ||
| OutlineGroup([task], | ||
| id: \.self, | ||
| children: \.visibleSubtasks) { maintask in |
There was a problem hiding this comment.
Ниже — только по делу, по текущему изменению и окружению:
-
Смена условия с hasSubtasks() на visibleSubtasks != nil меняет семантику. Если visibleSubtasks возвращает пустой массив вместо nil, условие сработает, вы пойдете в OutlineGroup, хотя реальных детей нет. Это может быть ок, но:
- Если вы осознанно используете Optional, чтобы различать “дети скрыты” (nil) и “детей нет” ([]), тогда явнее и безопаснее проверять не просто != nil, а что список не пуст: if let kids = task.visibleSubtasks, !kids.isEmpty { … }.
- Либо вообще уберите условие и всегда используйте OutlineGroup([task], children: .visibleSubtasks). Он сам корректно обработает отсутствие/пустоту детей. Это упростит код и исключит расхождения логики.
-
id: .self и в ForEach, и в OutlineGroup — потенциальная проблема. Если Task.hashValue зависит от изменяемых полей, вы получите нестабильную идентичность, сброс свернутости/развернутости и мерцание. Лучше использовать стабильный идентификатор: id: .id (или любой неизменяемый ключ).
-
Неочевидная асимметрия в фильтрах поиска:
- Для .completed вы фильтруете completed && parentTask == nil.
- Для незавершенных — только completed == false (без parentTask == nil).
Это может привести к дублям/несогласованности (например, показ подзадач как отдельных строк вне иерархии). Если это не намеренно — приведите обе ветки к единой логике (либо обе только корневые, либо обе включают подзадачи).
-
Сортировка детей. OutlineGroup берет порядок из visibleSubtasks. Убедитесь, что visibleSubtasks уже отсортированы (или сортируйте внутри свойства), иначе порядок может отличаться от TasksQuery.sortingWithCompleted для верхнего уровня.
-
Дублирование кода. Одинаковая ветка с OutlineGroup встречается минимум дважды. Вынесите отрисовку одного “task-узла” в отдельный View/функцию (например, TaskRow(task:)), чтобы не расходилась логика условий/идентификаторов/OutlineGroup между секциями.
-
Потенциальная производительность/чистота: вы несколько раз в body вызываете filter/sorted с похожими условиями. Если коллекции большие и данные не меняются ежесекундно, подумайте о мемоизации/предварительных вычислениях (или хотя бы о локальных let-переменных внутри body), чтобы избежать повторных одинаковых операций.
Короткая рекомендация по коду (если хотите минимальными правками и предсказуемым поведением):
- Заменить проверку на:
if let kids = task.visibleSubtasks, !kids.isEmpty { … }
или просто всегда использовать OutlineGroup без if. - Везде заменить id: .self на стабильный id.
- Привести фильтры в секции поиска к единой политике по parentTask.
| modelContext.delete(group) | ||
| } label: { | ||
| Image(systemName: "trash") | ||
| .foregroundStyle(Color.red) |
There was a problem hiding this comment.
Ниже — только содержательные замечания по сути изменения.
-
Семантика удаления: перенести modelContext.delete(group) за пределы цикла — корректно (раньше была попытка удалить один и тот же объект несколько раз). Дальше важно удостовериться в delete rule связи:
- Если у связи Group -> Project или обратной стоит .nullify, ручной проход и project.group = nil избыточен — можно удалять группу напрямую, SwiftData сам занулит инверс.
- Если стоит .deny, ваш ручной nullify перед удалением обязателен (как сейчас).
- Если стоит .cascade, текущий код предотвращает каскадное удаление проектов (возможно это и задумано). В любом случае лучше явно зафиксировать желаемое поведение в схеме.
-
Итерация по инверсной связи: вместо фильтрации всех projects логичнее и эффективнее пройтись по group.projects (если инверсия объявлена). Это уменьшит работу и исключит риск несоответствий при нескольких контекстах/экземплярах.
Пример:
for project in group.projects {
project.group = nil
}
modelContext.delete(group) -
Сохранение и обработка ошибок: после изменений стоит явно сохранить контекст и обработать возможную ошибку, если важна немедленная консистентность.
do {
try modelContext.save()
} catch {
// показать ошибку/откатить/логировать
} -
Согласованность контекста: убедитесь, что group и все project принадлежат одному и тому же modelContext. Иначе будут runtime-ошибки при установке связей/удалении. Можно добавить assert в отладке.
-
Сопоставление по идентификатору: если всё же оставляете фильтрацию, лучше сравнивать persistentModelID, чтобы избежать ловушек с разными экземплярами одной модели:
projects.filter { $0.group?.persistentModelID == group.persistentModelID } -
Разгрузка View: логику «разлинковать проекты и удалить группу» имеет смысл вынести в отдельный метод/сервис (например, в ViewModel/Interactor), чтобы упростить тестирование и переиспользование.
| if task.visibleSubtasks != nil { | ||
| OutlineGroup([task], | ||
| id: \.self, | ||
| children: \.visibleSubtasks) { maintask in |
There was a problem hiding this comment.
Ниже только то, что действительно может улучшить код и предотвратить регрессии.
-
Условие if task.visibleSubtasks != nil может пускать в ветку OutlineGroup задачи с пустым массивом дочерних (non-nil, но isEmpty). Это изменяет прежнюю семантику hasSubtasks() и может привести к «пустым» группам. Лучше:
- if let subs = task.visibleSubtasks, !subs.isEmpty { ... }
- Дополнительно, если visibleSubtasks в логике «видимости» уже означает «nil — нет видимых детей», то стоит гарантировать, что при пустом результате фильтра оно возвращает nil, а не [].
-
visibleSubtasks вызывается дважды (в условии и как children). Если это вычисляемое свойство с фильтрацией, вы делаете лишнюю работу. Сохраните в локальную константу:
- let subs = task.visibleSubtasks
- if let subs, !subs.isEmpty { OutlineGroup([task], …, children: .visibleSubtasks) … } else { … }
-
id: .self в ForEach/OutlineGroup — риск нестабильности идентификаторов при изменениях свойств, влияющих на Hashable, и проблем с диффингом. Предпочтительнее стабильный идентификатор:
- Сделайте Task: Identifiable или используйте явный .id и в OutlineGroup тоже id: .id.
-
Фильтры в ForEach сильно зашумляют и дублируются (searchResults.filter(...) внутри тернарного). Вынесите в отдельную переменную или computed property до ForEach. Это и читаемость, и меньше лишних пересчётов в body.
-
Логическая асимметрия фильтров:
- Для section == .completed вы фильтруете также по parentTask (root или относящиеся к mainTask), а для невыполненных — нет. Если это не специально, вы получите разный состав на разных секциях (возможны дубли/«потерянные» элементы). Если это специально — добавьте явный комментарий к этой ветке (или отразите в имени предиката), чтобы следующий разработчик не «починил» это «несоответствие».
-
OutlineGroup([task], …): если у вас список из многих задач, возможно выгоднее строить один OutlineGroup на уже отфильтрованном верхнем уровне (массив корневых задач), чем создавать OutlineGroup на каждый элемент. Это снизит вложенность/рендеринг и упростит код.
-
Если parentTask — это ссылочный тип (например, Core Data NSManagedObject), проверка ($0.parentTask == mainTask) может быть ненадёжной при разных контекстах/экземплярах. Лучше сравнивать по стабильному идентификатору (objectID или вашему id).
-
Мелочь: $0.completed == false замените на !$0.completed для единообразия и читабельности.
Пример рефакторинга фрагмента:
- Предикат и отфильтрованный список — до ForEach.
- Стабильные id.
- Без лишних вычислений visibleSubtasks.
- Защита от пустых детей.
Пример:
let filtered: [Task] = {
switch section {
case .completed:
return searchResults.filter {
$0.completed &&
($0.parentTaskId == nil || $0.parentTaskId == mainTask.id)
}
case .active:
return searchResults.filter { !$0.completed }
}
}()
ForEach(filtered, id: .id) { task in
if let subs = task.visibleSubtasks, !subs.isEmpty {
OutlineGroup([task], id: .id, children: .visibleSubtasks) { maintask in
// content
}
} else {
// одиночный элемент без группы
}
}
Если вы осознанно хотели завязать отображение групп именно на «видимых» детях (поиск/фильтр), то ключевой момент — договориться о семантике visibleSubtasks: nil означает «нет видимых детей», [] лучше не возвращать вовсе, чтобы и условие, и OutlineGroup вели себя согласованно.
| if task.visibleSubtasks != nil { | ||
| OutlineGroup([task], | ||
| id: \.self, | ||
| children: \.visibleSubtasks) { maintask in |
There was a problem hiding this comment.
Ниже — только предметные замечания по изменению hasSubtasks() → visibleSubtasks != nil и прилегающему коду.
-
Семантика условия. visibleSubtasks != nil не эквивалентно «есть видимые подзадачи», если visibleSubtasks иногда возвращает пустой массив, но не nil. Такое условие может переключить рендеринг в ветку OutlineGroup даже для «листа». Если цель — показывать OutlineGroup только при наличии дочерних элементов, проверьте и nil, и пустоту:
- if let subs = task.visibleSubtasks, !subs.isEmpty { … }
- Еще лучше: ввести var hasVisibleSubtasks: Bool и использовать его здесь и в других местах.
-
Возможно, условие вообще не нужно. OutlineGroup корректно работает и когда children возвращает nil (лист). Если UI для листа и узла одинакова, уберите if и всегда используйте OutlineGroup([task], children: .visibleSubtasks). Это упростит код и исключит расхождения поведения.
-
Идентичность элементов. id: .self для ForEach/OutlineGroup — потенциальный источник проблем (нестабильная идентичность при изменении свойств, дубликаты). Предпочтительнее:
- Если Task: Identifiable — уберите id: .self и используйте идентификатор модели.
- Для Core Data — id: .objectID.
- Аналогично внутри OutlineGroup.
-
Несогласованность фильтрации в секциях. Для .completed вы фильтруете только корневые задачи (parentTask == nil), а для незавершенных — нет. Это почти наверняка приводит к дублям: подзадача появляется и отдельно в списке, и внутри OutlineGroup родителя. Приведите условия к одному принципу (обычно берут только корневые на верхнем уровне, а вложенность раскрывает OutlineGroup):
- section == .completed ? results.filter { $0.completed && $0.parentTask == nil } : results.filter { !$0.completed && $0.parentTask == nil }
-
Повторная фильтрация в body. Сейчас фильтр строится внутри ForEach-ветвления. Вынесите вычисление filteredTasks для секции выше по дереву (или в computed var / ViewModel), чтобы:
- уменьшить дублирование и стоимость пересчетов;
- упростить чтение.
-
Мелочь по читабельности: используйте !task.completed вместо $0.completed == false.
-
Стабильный порядок. Если порядок элементов важен, добавьте сортировку после фильтра, иначе при изменениях данных SwiftUI может анимировать «перепрыгивания» строк.
-
Производительность visibleSubtasks. Убедитесь, что visibleSubtasks не делает тяжелых вычислений/запросов при каждом обновлении body. Если делает — кэшируйте/перенесите в модель, или сделайте ленивым.
Итоговая рекомендация по минимальным правкам, если UI листа и узла одинаков:
- Убрать if и всегда использовать OutlineGroup([task], id: .id (или .objectID), children: .visibleSubtasks).
- На верхнем уровне ForEach показывать только root-задачи (parentTask == nil).
- Заменить id: .self на стабильный id.
- Привести фильтры секций к симметрии и вынести их из ForEach.
| }, | ||
| "None" : { | ||
| "localizations" : { | ||
| "ar" : { |
There was a problem hiding this comment.
Ниже только предметные замечания и улучшения по добавленной строке в .xcstrings.
- Не используйте пользовательский текст как ключ. Ключ "No tasks for today" нестабилен: любое правка английской фразы поменяет ключ и сломает привязки. Предложение: заменить на стабильный семантический идентификатор, например:
- noTasksToday.title или tasks.emptyState.today
- Добавьте comment для переводчиков. Сейчас контекста нет — непонятно, это заголовок пустого состояния, подзаголовок или тост. Например:
- "comment": "Empty state title on the Tasks screen when there are 0 tasks for the current day."
- Согласуйте стиль с остальным каталогом:
- Регистр: Title Case vs sentence case. Сейчас "No tasks for today" — это sentence case. Если в проекте заголовки в Title Case, приведите к нему, либо наоборот.
- Пунктуация: обычно в UI без точки в конце. Проверьте соответствие общему гайду.
- Английский вариант лучше сократить до "No tasks today" — короче, в UI выглядит чище и чаще встречается. Меньше риска обрезки на узких экранах.
- Русский вариант: "Сегодня нет задач" звучит естественнее и короче, чем "Нет задач на сегодня". Рекомендую заменить, если не противоречит гайдам проекта.
- Терминология: убедитесь, что слово "задача/задачи" совпадает с глоссарием продукта (вдруг в интерфейсе везде "дела" или "таски").
- Поддержка множественного числа: если рядом есть строка вида "%d tasks today", лучше заменить две строки на одну pluralized-запись c нулевой формой, чтобы логика была централизована:
- en: zero: "No tasks today", one: "%d task today", other: "%d tasks today"
- ru: zero: "Сегодня нет задач", one: "Сегодня %d задача", few: "Сегодня %d задачи", many/other: "Сегодня %d задач"
Это можно сделать через plural variations в .xcstrings.
- Структурная гигиена каталога:
- Поддерживайте алфавитную сортировку ключей — меньше конфликтов в диффах.
- Если проект локализуется на больше языков, добавьте пустые записи или отметьте задачу для переводчиков, чтобы не забыть (иначе будет fallback на development language).
Итоговое предложение (пример):
"tasks.emptyState.today" : {
"comment": "Empty state title on the Tasks screen when there are 0 tasks for the current day.",
"localizations" : {
"en" : { "stringUnit" : { "state" : "translated", "value" : "No tasks today" } },
"ru" : { "stringUnit" : { "state" : "translated", "value" : "Сегодня нет задач" } }
}
}
Если в проекте уже есть plural для количества задач — лучше интегрировать нулевую форму туда и не добавлять отдельный ключ.
|
|
||
| func printInfo() { | ||
| let name = self.name | ||
| let uid = self.uid |
There was a problem hiding this comment.
Наблюдения по удаленному hasSubtasks():
Проблемы исходной реализации:
- Неоднозначная семантика. Если у родителя deletionDate == nil — считаем только «неделетнутые» подзадачи; если родитель удален — учитываем любые подзадачи. Это приводит к разным результатам в зависимости от состояния родителя и делает поведение метода неочевидным.
- Лишние вычисления. filter + count > 0 создает массив и проходит все элементы. Достаточно first(where:) или contains(_:) для раннего выхода.
- Сложность без необходимости. Двойная проверка на count > 0 в разных ветках, дублирование логики.
- Название не отражает критерий («любой сабтаск» vs «активный/видимый сабтаск»). Трудно понять, что именно ожидают от метода.
Рекомендации:
- Зафиксировать требуемую семантику и дать соответствующее имя. Если нужно «есть ли хотя бы одна не удаленная подзадача», то:
var hasActiveSubtasks: Bool {
subtasks?.contains { $0.deletionDate == nil } ?? false
}
Если родитель с deletionDate != nil должен обнулять смысл «активности», то:
var hasActiveSubtasks: Bool {
guard deletionDate == nil else { return false }
return subtasks?.contains { $0.deletionDate == nil } ?? false
}
Если нужна проверка «есть ли вообще подзадачи, независимо от статуса»:
var hasAnySubtasks: Bool {
!(subtasks?.isEmpty ?? true)
} - Сделать это вычисляемым свойством вместо метода — так читается лучше и не намекает на «тяжелое» действие.
- Если subtasks — Core Data relationship:
- Не приводите все в память для проверки. Используйте count-фетч с предикатом:
existsActiveSubtasks(todo: Todo) -> Bool {
// контекст и запрос зависят от вашей модели
// запрос на COUNT подзадач с deletionDate == nil для данного todo
} - Либо приводите к Set и используйте contains, избегая filter.
- Не приводите все в память для проверки. Используйте count-фетч с предикатом:
- По возможности сделайте subtasks не опциональным (пустой массив/Set вместо nil). Это упростит код и логику проверок.
- Покройте тестами граничные случаи:
- Нет подзадач
- Все подзадачи удалены
- Есть хотя бы одна активная
- Родитель удален (если это должно влиять)
Итого: удаление старого метода выглядит оправданным. Если функциональность нужна, верните ее как четко названное и простое вычисляемое свойство с ранним выходом, а не через filter + count. Если используете Core Data и важна производительность — предпочтительнее запрос на count с предикатом.
| CURRENT_PROJECT_VERSION = 111; | ||
| DEVELOPMENT_TEAM = 9Z68336878; | ||
| GENERATE_INFOPLIST_FILE = YES; | ||
| MACOSX_DEPLOYMENT_TARGET = 26.0; |
There was a problem hiding this comment.
Коротко: вы просто подняли CURRENT_PROJECT_VERSION с 109 до 111 в куче конфигураций. Само по себе — ок, но есть несколько рискованных мест и поводов для упрощения.
Что поправить прямо сейчас
- MACOSX_DEPLOYMENT_TARGET = 26.0 — это некорректное значение. Укажите реальную версию macOS SDK (например, 14.0/15.0) или удалите, чтобы наследовать от проекта. С таким значением сборка/валидация навернется.
- Убедитесь, что все бандлы в поставке (приложение, расширения, виджеты, Watch) имеют одинаковые CFBundleVersion/CFBundleShortVersionString. GENRERATE_INFOPLIST_FILE = YES подтянет CURRENT_PROJECT_VERSION, но для тех, у кого свой Info.plist, проверьте, что CFBundleVersion = $(CURRENT_PROJECT_VERSION), а не захардкожено число.
Риски/замечания
- Дублирование CURRENT_PROJECT_VERSION в каждом таргете. Это легко приводит к рассинхрону. Лучше:
- Вынести в проектный уровень или в общий .xcconfig;
- Удалить оверрайды из таргетов, чтобы они наследовали одно значение.
- IPHONEOS_DEPLOYMENT_TARGET = 18.1 — осознанный выбор? Это отрежет все устройства < 18.1. Если хотите 18.0 или ниже — скорректируйте. Также держите minOS единообразным среди приложения и экстеншенов.
- В некоторых конфигурациях задан CODE_SIGN_IDENTITY = "Apple Development". Для Release/App Store лучше не фиксировать identity вручную и оставить Automatic (или задать только для Debug), иначе возможны неожиданные подписи при CI.
Автоматизация, чтобы не ловить такие баги в будущем
- Централизация версий:
- Используйте agvtool new-version -all 111 (или xcrun agvtool) — он проставит CURRENT_PROJECT_VERSION во все таргеты.
- Храните CURRENT_PROJECT_VERSION и MARKETING_VERSION в общих .xcconfig. Удалите значения из таргетов.
- CI-проверка一致ности версий после сборки архива:
- Пример быстрой проверки внутри .xcarchive:
- Считать CFBundleVersion/CFBundleShortVersionString из всех Info.plist и сравнить.
- Скрипт-идея:
- Найти все Info.plist в Products внутри .xcarchive.
- Считать CFBundleVersion/CFBundleShortVersionString через /usr/libexec/PlistBuddy.
- Убедиться, что значения везде одинаковы.
- Пример быстрой проверки внутри .xcarchive:
- Пресняк на рассинхрон:
- В pre-commit/CI: запретить наличие прямых числовых значений CFBundleVersion в исходных Info.plist там, где должны быть $(CURRENT_PROJECT_VERSION).
- На уровне проекта: убрать per-target overrides для CURRENT_PROJECT_VERSION/ MARKETING_VERSION и MACOSX_DEPLOYMENT_TARGET, оставив их только в одном месте.
Итог
- Бамп версии — ок.
- Срочно исправьте MACOSX_DEPLOYMENT_TARGET = 26.0.
- Централизуйте версионирование и минимальные OS-версии, уберите дубли из таргетов.
- Добавьте простую CI-валидацию, чтобы больше не ловить несовпадения версий между бандлами.
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Ниже — только те замечания, которые реально улучшат код или устранят потенциальные проблемы.
-
Стабильные идентификаторы вместо .self
- Сейчас ForEach/OutlineGroup используют id: .self, .tag(maintask) и сравнение focus == maintask. Для моделей (SwiftData/CoreData) это хрупко: разные инстансы одной и той же записи не будут равны. Используйте стабильный ID (например, persistentModelID или собственный id).
- Пример:
- ForEach(tasks, id: .persistentModelID)
- OutlineGroup([task], id: .persistentModelID, children: .visibleSubtasks)
- .tag(task.persistentModelID)
- Сравнение: if focusTask.task?.persistentModelID == maintask.persistentModelID
-
Дублирование логики кнопок Play/Stop
- Блоки с кнопками полностью дублируются для maintask и task. Вынесите в отдельный вью/метод, принимающий идентификатор и модель задачи, и внутри инкапсулируйте логику запуска/остановки таймера и выставления focusTask. Это упростит поддержку и снизит риск рассинхронизации поведения между ветками.
-
Подозрительная логика с timer.reset() и проверкой режима
- В обработчике Stop сначала вызывается timer.reset(), а потом проверяется timer.mode == .pause || .longbreak — велика вероятность, что reset уже изменил mode и условие никогда не выполнится. Сохраните предыдущее состояние до reset или поменяйте порядок:
- let wasBreak = timer.mode == .pause || timer.mode == .longbreak
- timer.reset()
- if wasBreak { timer.skip() }
- В обработчике Stop сначала вызывается timer.reset(), а потом проверяется timer.mode == .pause || .longbreak — велика вероятность, что reset уже изменил mode и условие никогда не выполнится. Сохраните предыдущее состояние до reset или поменяйте порядок:
-
Сравнение по объектам вместо по ID
- Уже отмечено выше, но отдельно подчеркну: focus == task/maintask ненадёжно для моделей. Перейдите на сравнение по стабильным идентификаторам везде: в .tag, в selection, в условии показа Stop/Play.
-
Размещение listRowSeparator(.hidden)
- Применять .listRowSeparator к TaskRowView внутри HStack и к самому OutlineGroup — сомнительно. Корректнее навешивать модификатор на корневой вью строки (тот, что возвращает тело строки, например ваш HStack). Тогда поведение предсказуемо как для обычных строк, так и внутри OutlineGroup:
- OutlineGroup { node in
row(for: node) // HStack
.listRowSeparator(.hidden)
}
- OutlineGroup { node in
- Применять .listRowSeparator к TaskRowView внутри HStack и к самому OutlineGroup — сомнительно. Корректнее навешивать модификатор на корневой вью строки (тот, что возвращает тело строки, например ваш HStack). Тогда поведение предсказуемо как для обычных строк, так и внутри OutlineGroup:
-
Проверка пустоты коллекции
- if tasksTodayActive.count > 0 замените на более идиоматичное if !tasksTodayActive.isEmpty.
-
Пустое состояние: сохраняйте стабильную структуру
- Сейчас при пустом списке вы возвращаете VStack вместо List. Это приводит к «прыгающему» layout/стилю и может ломать selection-поведение. Лучше всегда возвращать List, а пустое состояние показывать через .overlay или ContentUnavailableView (iOS 17+):
- List(selection: $selectedTask) { ... }
.overlay {
if tasksTodayActive.isEmpty { EmptyStateView() }
}
- List(selection: $selectedTask) { ... }
- Сейчас при пустом списке вы возвращаете VStack вместо List. Это приводит к «прыгающему» layout/стилю и может ломать selection-поведение. Лучше всегда возвращать List, а пустое состояние показывать через .overlay или ContentUnavailableView (iOS 17+):
-
Сортировка в @query вместо .sorted в body
- Пересортировка в body будет происходить на каждый ререндер. Перенесите сортировку в @query(sort:) через SortDescriptor (или объедините с TasksQuery.defaultSorting на уровне запроса). Это снизит нагрузку и уменьшит «шум» в диффах UI.
-
Магическое число viewMode = 1
- Замените на enum с понятными кейсами. Это упростит чтение и снизит риск ошибок:
- enum ViewMode { case list, focus }
- @State private var viewMode: ViewMode = .list
- viewMode = .focus
- Замените на enum с понятными кейсами. Это упростит чтение и снизит риск ошибок:
-
Доступность и тестируемость
- Идентификатор для stop-кнопки отсутствует, для play — есть. Синхронизируйте: добавьте .accessibilityIdentifier и для Stop. Также используйте стабильные ID в идентификаторах (например, persistentModelID.uuidString), а не имя задачи:
- .accessibilityIdentifier("task-(task.persistentModelID)-play")
- .accessibilityIdentifier("task-(task.persistentModelID)-stop")
- Параллельно добавьте .accessibilityLabel и .accessibilityHint — это улучшит UX с VoiceOver.
- Идентификатор для stop-кнопки отсутствует, для play — есть. Синхронизируйте: добавьте .accessibilityIdentifier и для Stop. Также используйте стабильные ID в идентификаторах (например, persistentModelID.uuidString), а не имя задачи:
-
Цвета в empty state
- .foregroundStyle(Color.gray) замените на .foregroundStyle(.secondary), чтобы корректно адаптироваться к темам/доступности.
-
Кнопки внутри List
- Если таргетите macOS/iPadOS c selection в List, стоит явно указать стиль, чтобы нажатие на кнопки не конфликтовало с выбором строки:
- .buttonStyle(.borderless)
- Если таргетите macOS/iPadOS c selection в List, стоит явно указать стиль, чтобы нажатие на кнопки не конфликтовало с выбором строки:
-
Семантика hasSubtasks() → visibleSubtasks?.isEmpty == false
- Вы изменили критерий с «есть подзадачи» на «есть видимые подзадачи». Если фильтрация «невидимых» — осознанное требование, ок. Если нет — есть риск скрыть родителя с невидимыми подзадачами и изменить UX разворачивания. Уточните, что именно хотите показать.
Итоговая «быстрая выжимка» того, что стоит сделать в первую очередь:
- Перейти на стабильные ID в ForEach/OutlineGroup/.tag и сравнениях.
- Вынести Play/Stop в переиспользуемый компонент и поправить логику reset/skip.
- Держать всегда List и показывать пустое состояние через overlay.
- Перенести сортировку в @query.
- Убрать .listRowSeparator с дочерних сабвью и повесить на корневой HStack строки.
- Заменить viewMode = 1 на enum и поправить a11y-идентификаторы.
| if task.visibleSubtasks?.isEmpty == false { | ||
| OutlineGroup([task], | ||
| id: \.self, | ||
| children: \.visibleSubtasks) { maintask in |
There was a problem hiding this comment.
Наблюдения по изменению if task.hasSubtasks() -> if task.visibleSubtasks?.isEmpty == false:
-
Семантика. hasSubtasks(), скорее всего, проверяет существование любых подзадач, а visibleSubtasks — только «видимых». В результате задача с «скрытыми» подзадачами больше не будет рендериться как OutlineGroup. Если это намеренно — ок, но убедитесь, что логика проверки и источник children совпадают по фильтрам, иначе возможны расхождения (например, подзадачи с другим статусом попадут/не попадут непоследовательно).
-
Читаемость и повторные вычисления. Вы сейчас дважды обращаетесь к visibleSubtasks (в if и через keyPath в OutlineGroup). Если это вычисляемое свойство с фильтрацией/запросом, вы платите за это дважды. Лучше:
- Вынести проверку в вычисляемое свойство модели: var hasVisibleSubtasks: Bool { !(visibleSubtasks?.isEmpty ?? true) }
- Или вьюшке кэшировать результат: if let subtasks = task.visibleSubtasks, !subtasks.isEmpty { ... }
Оба варианта читаются лучше, чем сравнение с == false, и избегают двойной работы.
-
Идентичность элементов. id: .self требует стабильного и неизменного Hashable для Task. Если равенство/хэш зависят от мутируемых полей, вы получите глитчи (сброс анимаций, состояний раскрытия OutlineGroup и т.д.). Рекомендации:
- Если Task: Identifiable — используйте инициализатор без id-параметра: OutlineGroup([task], children: .visibleSubtasks) { ... }.
- Иначе используйте стабильный идентификатор (например, id: .id), а не .self.
-
Сортировка потомков. Корневые задачи вы сортируете TasksQuery.sortingWithCompleted, а вот visibleSubtasks внутри OutlineGroup — нет (если только visibleSubtasks сама не возвращает отсортированный массив). Чтобы дерево было предсказуемым, обеспечьте сортировку и на уровне потомков.
-
Консистентность статусов в колонке. Если колонка — это статус, а visibleSubtasks возвращает подзадачи с «чужими» статусами, вы начнёте смешивать статусы внутри одной колонки. Либо фильтруйте потомков по статусу, либо явно закрепите правило (например, показываем только подзадачи с тем же статусом, что и родитель).
-
Нулевые/пустые дети. Для OutlineGroup children можно возвращать nil при отсутствии детей — это избавит от внешней проверки вообще. То есть дайте visibleSubtasks возвращать nil, когда подзадач нет, и можно убрать if. Если вам важно иметь другой UI для «листа» vs «узла», тогда оставляйте проверку, но используйте вариант с if let.
-
Потенциальная рекурсия/циклы. Если visibleSubtasks строятся на базе ссылок parent/child, убедитесь, что нет возможных циклов (иначе OutlineGroup зациклится). Это скорее защита на уровне модели.
Пример более чистого фрагмента:
-
Если Task: Identifiable
if let subtasks = task.visibleSubtasks, !subtasks.isEmpty {
OutlineGroup([task], children: .visibleSubtasks) { maintask in
...
}
} else {
...
} -
Или через вычисляемое свойство модели
if task.hasVisibleSubtasks {
OutlineGroup([task], children: .visibleSubtasks) { maintask in
...
}
} else {
...
}
Главные рекомендации к внедрению:
- Централизовать критерии видимости/фильтрации/сортировки в одном месте (в модели или слое запроса), чтобы проверка наличия детей и сами дети были согласованными.
- Перейти с id: .self на стабильный идентификатор (или Identifiable).
- Убедиться, что visibleSubtasks уже отсортированы нужным образом.
| if task.visibleSubtasks?.isEmpty == false { | ||
| OutlineGroup([task], | ||
| id: \.self, | ||
| children: \.visibleSubtasks) { maintask in |
There was a problem hiding this comment.
Ниже — только те замечания, которые действительно могут улучшить код или предотвратить проблемы.
-
Семантика изменения hasSubtasks() → visibleSubtasks?.isEmpty == false:
• Это меняет UI-логику: задачи с “невидимыми”/отфильтрованными подзадачами теперь считаются листовыми и не оборачиваются в OutlineGroup. Убедитесь, что это намеренно (это влияет на UX и состояние раскрытия).
• Выражение с опционалом и == false читается хуже и может путать. Лучше явное и безопасное: if let children = task.visibleSubtasks, !children.isEmpty { … }. И/или добавьте вычисляемое свойство в модель: var hasVisibleSubtasks: Bool { !(visibleSubtasks?.isEmpty ?? true) } и используйте его в обоих местах. -
Дублирующиеся вычисления visibleSubtasks:
• Сейчас вы считаете видимые подзадачи дважды: в if и потом снова через keyPath в OutlineGroup. Если visibleSubtasks — затратный вычисляемый геттер (фильтрация/сортировка), это лишняя работа. Варианты:
– Вообще убрать предварительную проверку и всегда использовать OutlineGroup: он корректно обрабатывает листы (nil/пустой children) без индикатора раскрытия.
– Если нужна разная разметка для листов и групп, кэшируйте вычисление на уровне модели/VM (или как минимум в локальную переменную для условия, а для OutlineGroup — подумайте, действительно ли нужно опираться на keyPath, если children уже вычислены). -
Идентификаторы в ForEach/OutlineGroup:
• id: .self рискован, если self не является стабильным идентификатором между перестроениями (особенно для NSManagedObject/CoreData). Это ведет к неправильным диффам, мерцаниям и сбросу состояния раскрытия. Рекомендуется id: .id (постоянный UUID/объектный ID) и соответствие Identifiable. -
Структура дерева и сортировка:
• OutlineGroup не сортирует детей. Убедитесь, что visibleSubtasks уже отсортированы согласованно с TasksQuery.sortingWithCompleted, иначе порядок в дереве и списке будет различаться.
• Добавьте защиту от потенциальных циклов parentTask/visibleSubtasks (даже если «не должно быть»): хотя бы на уровне формирования visibleSubtasks. -
Архитектура/производительность:
• Сейчас на каждый элемент с подзадачами создается свой OutlineGroup. Это увеличивает глубину и сложность вью-дерева. Обычно проще и легче для SwiftUI создать один OutlineGroup на коллекцию корневых задач в секции:
OutlineGroup(rootTasksForStatus(status), children: .visibleSubtasks) { task in
TaskRow(task)
}
Это уберет условную обертку и уменьшит количество OutlineGroup.
• Если оставляете текущий подход, вынесите рендеринг строки задачи (и проверку на подзадачи) в отдельный subview/функцию, чтобы не дублировать блоки в двух местах. -
Стабильность состояния раскрытия:
• Состояние раскрытия в OutlineGroup завязано на идентичность элементов. Переход на стабильные id (см. выше) критичен, иначе пользователи будут терять раскрытое/свернутое состояние при обновлениях списка. -
Стиль/читаемость:
• visibleSubtasks?.isEmpty == false — хрупкая запись. Лучше if let children = …, !children.isEmpty или отдельное свойство hasVisibleSubtasks. Это одновременно делает код очевиднее и избавляет от сравнений опционала с false.
Если кратко, основные практические шаги:
- Перейти на стабильные идентификаторы (id: .id).
- Заменить условие на hasVisibleSubtasks или явную распаковку.
- Рассмотреть один OutlineGroup на секцию вместо OutlineGroup на каждый элемент.
- Убедиться в согласованной сортировке children и намеренности смены семантики «видимость vs наличие» подзадач.
| if task.visibleSubtasks?.isEmpty == false { | ||
| OutlineGroup([task], | ||
| id: \.self, | ||
| children: \.visibleSubtasks) { maintask in |
There was a problem hiding this comment.
Наблюдения по изменению и коду вокруг него:
-
Семантическая разница: переход с hasSubtasks() на visibleSubtasks?.isEmpty == false меняет критерий показа OutlineGroup. Если visibleSubtasks скрывает дочерние (например, по поиску/фильтрам), узлы с реальными подзадачами, но без «видимых» детей перестанут рендериться через OutlineGroup и попадут в else-ветку. Это может привести к непоследовательному UX (контейнер «превращается» в лист при смене фильтра). Если цель — именно так себя вести, ок. Если нет — лучше проверять «наличие фактических подзадач» и доверить OutlineGroup отображение текущей видимости.
-
Лишняя работа и повторные вычисления: visibleSubtasks, скорее всего, вычисляется (фильтруется) каждый раз. Сейчас вы считаете ее в if, а затем OutlineGroup снова дергает ее через keyPath. Чтобы не дублировать работу, либо:
- уберите if и всегда используйте OutlineGroup — он сам не покажет disclosure при отсутствии детей;
- либо вычисляйте children один раз и рендерьте без keyPath (но стандартный OutlineGroup требует keyPath/коллекцию, так что проще убрать if).
-
Читаемость: if task.visibleSubtasks?.isEmpty == false работает, но менее читаемо. Лаконичнее и однозначнее:
- if let subtasks = task.visibleSubtasks, !subtasks.isEmpty { ... }
-
Идентификаторы: id: .self для ForEach и OutlineGroup — хрупко, если Hashable завязан на изменяемые поля или на ссылочную идентичность. Лучше стабильный id поля модели (например, id: .id). Это уменьшит вероятность глитчей при обновлениях и перестановках.
-
Асимметрия фильтрации по section: для .completed вы дополнительно фильтруете по parentTask (nil или == mainTask), а для незавершенных — нет. Похоже на логическую несогласованность: незавершенные могут включать задачи вне контекста mainTask. Уточните намерение; если секции должны быть сопоставимы, добавьте такой же фильтр в ветку незавершенных.
-
Микро-замечание: $0.completed == false лучше записать как !$0.completed.
-
Упрощение структуры:
- Подготовьте tasksForSection заранее (с поиском/фильтрацией), чтобы не дублировать filter внутри ForEach.
- Внутри ForEach всегда используйте OutlineGroup; для листьев можно варьировать контент прямо в content замыкании:
Пример направления:
- Предварительно: let tasks = section == .completed ? searchResults.filter { $0.completed && ($0.parentTask == nil || $0.parentTask == mainTask) } : searchResults.filter { !$0.completed && ($0.parentTask == nil || $0.parentTask == mainTask) }
- В ForEach(tasks, id: .id) {
OutlineGroup([$0], id: .id, children: .visibleSubtasks) { node in
// Внутри при необходимости отличайте лист/контейнер через (node.visibleSubtasks?.isEmpty == false)
}
}
Итог: текущее изменение допустимо, если вы намеренно хотите скрывать «контейнерность» узла при пустой видимости детей. Если нет — лучше убрать предварительную проверку вовсе и оставить решать OutlineGroup, одновременно избежав двойных вычислений и улучшив производительность/простоту. Обязательно рассмотрите переход на стабильный id и выравнивание логики фильтра по секциям.
| if task.visibleSubtasks?.isEmpty == false { | ||
| OutlineGroup([task], | ||
| id: \.self, | ||
| children: \.visibleSubtasks) { maintask in |
There was a problem hiding this comment.
Ниже — только те замечания, которые реально повлияют на корректность, читаемость и поведение.
-
Изменение семантики: переход с hasSubtasks() на visibleSubtasks?.isEmpty == false меняет UX. Теперь задача не считается «группой», если подзадачи существуют, но сейчас не видимы (отфильтрованы). Это может убирать «раскрываемость» узла при активном поиске/фильтре. Проверьте, что это желаемо. Если нет — оставьте проверку по полным подзадачам или введите явный флаг:
- var hasVisibleSubtasks: Bool { !(visibleSubtasks?.isEmpty ?? true) }
- и используйте его в одном месте.
-
Идентичность элементов: id: .self в ForEach и OutlineGroup — потенциальный источник глючного диффинга при изменении полей задачи (title, completed и т. п.). Лучше стабильный id:
- если Task: Identifiable — уберите параметр id;
- иначе используйте id: .id (UUID).
-
Дублирование и логическая ошибка фильтрации в ForEach:
- Для .completed вы фильтруете только корневые (parentTask == nil), а для «активных» — нет. Это почти наверняка приводит к дублям (подзадача попадёт и как отдельный элемент, и как дочерний в OutlineGroup).
- Исправьте на симметричное условие и упростите логику:
let roots = searchResults
.filter { $0.parentTask == nil }
.filter { section == .completed ? $0.completed : !$0.completed }
-
Снижение шума и накладных расходов в body:
- Сейчас вы дважды вызываете filter в тернарном операторе. Вынесите фильтрацию в переменную/компьютед или в модель.
- Избавьтесь от $0.completed == false в пользу !$0.completed.
-
Лишняя обёртка OutlineGroup на каждый элемент:
- Оптимальнее и чище отдать в один OutlineGroup сразу массив корневых элементов. Это убирает условие if и упрощает код:
OutlineGroup(roots, id: .id, children: .visibleSubtasks) { task in
TaskRow(task: task)
} - Если для «листов» и «узлов» нужен разный внешний вид, вычисляйте признак внутри content:
let isGroup = !(task.visibleSubtasks?.isEmpty ?? true)
- Оптимальнее и чище отдать в один OutlineGroup сразу массив корневых элементов. Это убирает условие if и упрощает код:
-
Чтение optional в условии:
- if task.visibleSubtasks?.isEmpty == false хуже читается и даёт шанс на ошибки. Предпочтительно:
if let subtasks = task.visibleSubtasks, !subtasks.isEmpty { ... }
или
if !(task.visibleSubtasks?.isEmpty ?? true) { ... }
- if task.visibleSubtasks?.isEmpty == false хуже читается и даёт шанс на ошибки. Предпочтительно:
-
Производительность visibleSubtasks:
- Если visibleSubtasks — дорогой фильтр (особенно зависящий от поисковой строки), кэшируйте/готовьте его в модели, чтобы не пересчитывать на каждый ререндер.
Итоговая форма секции с учётом правок может выглядеть так:
- заранее посчитать roots по разделу (только parentTask == nil);
- один OutlineGroup с children: .visibleSubtasks;
- стабильные id;
- без дополнительных if перед OutlineGroup.
| return self.subtasks ?? [] | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Ниже только по делу — где логика «течёт», что может сломать UI, и как минимально, но правильно поправить.
Основные проблемы
-
Нарушение контракта для OutlineGroup: в ветке deletionDate != nil вы возвращаете self.subtasks ?? [], то есть не nil при отсутствии детей. Комментарий прямо говорит: «Возвращаем nil, если детей нет — так OutlineGroup не будет считать, что ветка раскрываемая». Сейчас ветка станет «раскрываемой» даже при пустом массиве (chevron раскроется в пустоту). Это регрессия.
-
Несогласованная семантика visibleSubtasks. Имя говорит «видимые» (= не удалённые) подзадачи, но для удалённой задачи вы возвращаете все подзадачи, включая удалённые. Это вводит в заблуждение. Если именно так и задумано (например, для экрана корзины), лучше либо:
- переименовать свойство (например, outlineChildren или childrenForDisplay),
- либо чётко параметризовать контекст (includeDeleted или inTrash), а не привязываться к состоянию родителя.
-
Неявное оборачивание в Optional.some в else-ветке маскирует ошибку: возвращая [] вместо nil вы меняете поведение, и это неочевидно при беглом просмотре.
-
Потенциальный снос API: вы удалили hasSubtasks(). Если он где-то используется — будет падение компиляции. Если цель — заменить, дайте короткий эквивалент через visibleSubtasks != nil.
Рекомендации и правки
- Исправить возврат для случая удалённого родителя: возвращать nil при отсутствии детей (аналогично первой ветке). Заодно убрать лишние self и дубли.
Вариант без смены семантики имени (сохраняем вашу идею «в корзине показывать всех детей»), но корректно обрабатываем пустоту:
var visibleSubtasks: [Todo]? {
let children: [Todo]
if deletionDate == nil {
children = (subtasks ?? []).filter { $0.deletionDate == nil }
} else {
children = subtasks ?? []
}
return children.isEmpty ? nil : children
}
- Если всё-таки «visible» должно значить «не удалённые» при любых условиях, уберите зависимость от deletionDate родителя:
var visibleSubtasks: [Todo]? {
let children = (subtasks ?? []).filter { $0.deletionDate == nil }
return children.isEmpty ? nil : children
}
И отдельно сделайте «дети для корзины»:
var allSubtasksForTrash: [Todo]? {
let children = subtasks ?? []
return children.isEmpty ? nil : children
}
- Если вы избавляетесь от hasSubtasks(), предложите лаконичную замену:
var hasVisibleSubtasks: Bool { visibleSubtasks != nil }
(Или сохраните старое имя, если оно используется в кодовой базе.)
- Чуть-чуть DRY и читаемость:
- Введите вспомогательное:
private var allSubtasks: [Todo] { subtasks ?? [] }
и используйте его в вычислениях, чтобы не повторять (subtasks ?? []).
- Тесты (очень желательно, чтобы не ловить регрессии в UI):
- Родитель не удалён, детей нет → visibleSubtasks == nil.
- Родитель не удалён, есть только удалённые дети → visibleSubtasks == nil.
- Родитель не удалён, есть живые дети → visibleSubtasks?.count > 0, только «живые».
- Родитель удалён, детей нет → visibleSubtasks == nil (или не nil, если выберете вариант для корзины, но тогда это должно быть осознанно).
- Родитель удалён, есть дети → visibleSubtasks?.count > 0 (и, если это «режим корзины», включаем и удалённых).
Итог
- Главная техническая правка: вернуть логику nil при отсутствии детей в обеих ветках.
- Семантику «visible» либо привести к названию (всегда фильтровать удалённые), либо назвать по назначению/контексту.
- hasSubtasks заменить на computed bool через visibleSubtasks != nil (или убедиться, что нигде не используется).
| CURRENT_PROJECT_VERSION = 112; | ||
| DEVELOPMENT_TEAM = 9Z68336878; | ||
| GENERATE_INFOPLIST_FILE = YES; | ||
| MACOSX_DEPLOYMENT_TARGET = 26.0; |
There was a problem hiding this comment.
Коротко по делу — что стоит поправить и как упростить обслуживание версии:
-
Единый источник правды для CURRENT_PROJECT_VERSION. Сейчас значение продублировано во множестве конфигураций и таргетов — это легко пропустить в следующем PR. Вынесите CURRENT_PROJECT_VERSION на уровень проекта или в общую .xcconfig, а в таргетах уберите переопределения. Добавьте VERSIONING_SYSTEM = apple-generic и используйте agvtool/CI для автоинкремента.
-
Автоматизация инкремента. Настройте одно из:
- agvtool new-version -all , при условии VERSIONING_SYSTEM = apple-generic и единственного определения CURRENT_PROJECT_VERSION.
- fastlane increment_build_number или собственный скрипт в CI, который обновляет .xcconfig/проект.
-
Валидация на CI. Добавьте проверку, что все пакуемые таргеты (приложение + все extensions/widgets/watch) имеют одинаковые CFBundleVersion. Пример быстрой проверки:
- plutil -extract CFBundleVersion raw <Built-Info.plist> и сравнить значения по списку таргетов.
- Либо grep по .pbxproj на наличие старых значений, чтобы PR падал, если что-то забыто.
-
GENERATE_INFOPLIST_FILE vs INFOPLIST_FILE. В ряде таргетов установлены оба. Определитесь:
- Либо генерируете Info.plist (GENERATE_INFOPLIST_FILE = YES) и не храните INFOPLIST_FILE.
- Либо используете собственный Info.plist и удаляете ключ GENERATE_INFOPLIST_FILE. Это снизит путаницу, где берется CFBundleVersion.
-
Дублирование CFBundleVersion в кастомных Info.plist. Если где-то остаются ручные Info.plist, убедитесь, что в них нет статического CFBundleVersion — иначе можно незаметно переехать поверх значения из CURRENT_PROJECT_VERSION.
-
Несогласованность deployment targets:
- MACOSX_DEPLOYMENT_TARGET = 26.0 выглядит некорректно (на сегодня таких версий macOS нет). Вероятная опечатка/копипаст — проверьте и исправьте, иначе возможны ошибки сборки/валидации.
- IPHONEOS_DEPLOYMENT_TARGET = 18.1 задан у части таргетов (например, extensions). Убедитесь, что главный iOS‑таргет не имеет более низкого min OS, иначе расширение не пройдет в бандл. Лучше зафиксировать единый min target для связки “приложение + расширения”.
-
Лишние/платформенные флаги:
- ENABLE_APP_SANDBOX в iOS‑таргете не нужен (sandbox на iOS всегда включен). Для чистоты конфигурации удалите неактуальные ключи.
-
Уменьшение “шума” в .pbxproj. Помимо вынесения версии в .xcconfig, стоит вынести MARKETING_VERSION и DEVELOPMENT_TEAM на уровень проекта, чтобы не дублировать в каждом таргете.
Итог: сам bump с 109 до 112 — ок, но текущая схема поддержки версии трудоемкая и склонна к ошибкам. Вынесение версии в один источник + автоматизация + санитация build settings решат большинство рисков на будущие релизы.
| } | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Ниже только по делу — где есть смысл улучшить или что может сломаться.
Что настораживает
- Нарушение контракта, описанного в комментарии. Комментарий обещает: возвращаем только «живые» подзадачи. Новая ветка else возвращает все подзадачи для удалённого родителя, включая удалённые. Это прямое расхождение со спецификацией и потенциальный источник багов в UI (OutlineGroup может показывать индикатор раскрытия для веток, где все дети удалены).
- Скрытое изменение семантики. Раньше visibleSubtasks всегда фильтровал детей по deletionDate == nil. Теперь результат зависит от состояния родителя. Если это сознательное решение — обязательно обновите док-комментарий. Если нет — верните однозначную семантику.
- Удалён hasSubtasks() без замены. Если были вызовы hasSubtasks(), они или сломаются, или переедут на visibleSubtasks != nil, что изменит поведение (особенно для удалённых родителй, см. выше).
- Избыточная ветвистость и стиль. Ветка else с if let subtasks, subtasks.isEmpty == false избыточна и читается тяжелее, чем guard/тернарный оператор. Также subtasks?.filter { … } ?? [] предпочтительнее, чем (subtasks ?? []).filter { … }.
Рекомендации
- Определиться с контрактом видимости.
- Вариант A (строго по комментарию): всегда показываем только «живые» подзадачи, независимо от статуса родителя.
- Вариант B (особый режим для удалённых родителй): тогда явно задокументируйте это в комментарии и названии (например, visibleSubtasksConsideringParentDeletion).
- Если хотите оставить текущую идею «для удалённого родителя показываем всех детей», упростите и задекларируйте это:
var visibleSubtasks: [Todo]? {
let children = subtasks ?? []
let result = (deletionDate == nil)
? children.filter { $0.deletionDate == nil }
: children
return result.isEmpty ? nil : result
}
И обновите комментарий: «Если родитель удалён, возвращаем всех детей; иначе — только живых». - Если цель — вернуть изначальный контракт (только живые дети), сделайте так:
var visibleSubtasks: [Todo]? {
let filtered = subtasks?.filter { $0.deletionDate == nil } ?? []
return filtered.isEmpty ? nil : filtered
} - Возврат признака наличия детей:
- Если hasSubtasks() больше не нужен — удаляйте и заменяйте на visibleSubtasks != nil.
- Если нужен отдельно — сделайте свойство и используйте contains(where:) без лишних аллокаций:
var hasVisibleSubtasks: Bool {
guard deletionDate == nil else { return false } // или нужная вам логика
return subtasks?.contains { $0.deletionDate == nil } == true
}
- Стиль/читаемость:
- Замените subtasks.isEmpty == false на !subtasks.isEmpty.
- Уберите self. там, где он не нужен.
- Избегайте дублирования фильтра и множественных веток — вычисляйте итоговый массив один раз, потом приводите к nil, если пусто.
Побочные эффекты, о которых стоит помнить
- Изменение логики для удалённых родителй повлияет на раскрываемость OutlineGroup: ветка может считаться раскрываемой даже если все дочерние — «мёртвые». Если это нежелательно, не возвращайте не‑живых детей ни при каких условиях.
- Если родительские удалённые элементы в принципе не отображаются в OutlineGroup, ветка else никогда не сработает — тогда этот код только усложняет понимание и его лучше убрать.
Итог
- Приведите поведение visibleSubtasks в соответствие с контрактом и/или обновите комментарий.
- Упростите логику вычисления и избавьтесь от неоднозначности с удалёнными родителями.
- Если удалили hasSubtasks(), замените его на явное свойство или на проверку visibleSubtasks != nil в местах использования.
No description provided.