Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 20 additions & 20 deletions PomPadDo.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -1126,7 +1126,7 @@
CLANG_ENABLE_MODULES = YES;
CODE_SIGN_ENTITLEMENTS = PomPadDoWidgetsExtension.entitlements;
CODE_SIGN_STYLE = Automatic;
CURRENT_PROJECT_VERSION = 109;
CURRENT_PROJECT_VERSION = 110;
GENERATE_INFOPLIST_FILE = YES;
INFOPLIST_FILE = "PomPadDoWatchWidgetsExtension Info.plist";
INFOPLIST_KEY_CFBundleDisplayName = PomPadDoWidgets;
Expand Down Expand Up @@ -1163,7 +1163,7 @@
CLANG_ENABLE_MODULES = YES;
CODE_SIGN_ENTITLEMENTS = PomPadDoWidgetsExtension.entitlements;
CODE_SIGN_STYLE = Automatic;
CURRENT_PROJECT_VERSION = 109;
CURRENT_PROJECT_VERSION = 110;
GENERATE_INFOPLIST_FILE = YES;
INFOPLIST_FILE = "PomPadDoWatchWidgetsExtension Info.plist";
INFOPLIST_KEY_CFBundleDisplayName = PomPadDoWidgets;
Expand Down Expand Up @@ -1195,7 +1195,7 @@
isa = XCBuildConfiguration;
buildSettings = {
CODE_SIGN_STYLE = Automatic;
CURRENT_PROJECT_VERSION = 109;
CURRENT_PROJECT_VERSION = 110;
DEVELOPMENT_TEAM = 9Z68336878;
GENERATE_INFOPLIST_FILE = YES;
MARKETING_VERSION = 1.0;
Expand All @@ -1217,7 +1217,7 @@
isa = XCBuildConfiguration;
buildSettings = {
CODE_SIGN_STYLE = Automatic;
CURRENT_PROJECT_VERSION = 109;
CURRENT_PROJECT_VERSION = 110;
DEVELOPMENT_TEAM = 9Z68336878;
GENERATE_INFOPLIST_FILE = YES;
MARKETING_VERSION = 1.0;
Expand All @@ -1243,7 +1243,7 @@
ASSETCATALOG_COMPILER_GLOBAL_ACCENT_COLOR_NAME = AccentColor;
CODE_SIGN_ENTITLEMENTS = PomPadDo.mobile/PomPadDo_mobile.entitlements;
CODE_SIGN_STYLE = Automatic;
CURRENT_PROJECT_VERSION = 109;
CURRENT_PROJECT_VERSION = 110;
DEVELOPMENT_ASSET_PATHS = "\"PomPadDo.mobile/Preview Content\"";
ENABLE_APP_SANDBOX = YES;
ENABLE_PREVIEWS = YES;
Expand Down Expand Up @@ -1284,7 +1284,7 @@
ASSETCATALOG_COMPILER_GLOBAL_ACCENT_COLOR_NAME = AccentColor;
CODE_SIGN_ENTITLEMENTS = PomPadDo.mobile/PomPadDo_mobile.entitlements;
CODE_SIGN_STYLE = Automatic;
CURRENT_PROJECT_VERSION = 109;
CURRENT_PROJECT_VERSION = 110;
DEVELOPMENT_ASSET_PATHS = "\"PomPadDo.mobile/Preview Content\"";
ENABLE_APP_SANDBOX = YES;
ENABLE_PREVIEWS = YES;
Expand Down Expand Up @@ -1323,7 +1323,7 @@
isa = XCBuildConfiguration;
buildSettings = {
CODE_SIGN_STYLE = Automatic;
CURRENT_PROJECT_VERSION = 109;
CURRENT_PROJECT_VERSION = 110;
GENERATE_INFOPLIST_FILE = YES;
IPHONEOS_DEPLOYMENT_TARGET = 18.1;
MARKETING_VERSION = 1.0;
Expand All @@ -1341,7 +1341,7 @@
isa = XCBuildConfiguration;
buildSettings = {
CODE_SIGN_STYLE = Automatic;
CURRENT_PROJECT_VERSION = 109;
CURRENT_PROJECT_VERSION = 110;
GENERATE_INFOPLIST_FILE = YES;
IPHONEOS_DEPLOYMENT_TARGET = 18.1;
MARKETING_VERSION = 1.0;
Expand All @@ -1363,7 +1363,7 @@
ASSETCATALOG_COMPILER_GLOBAL_ACCENT_COLOR_NAME = AccentColor;
CODE_SIGN_ENTITLEMENTS = "PomPadDo.watch Watch App/PomPadDo.watch Watch App.entitlements";
CODE_SIGN_STYLE = Automatic;
CURRENT_PROJECT_VERSION = 109;
CURRENT_PROJECT_VERSION = 110;
DEVELOPMENT_ASSET_PATHS = "\"PomPadDo.watch Watch App/Preview Content\"";
ENABLE_PREVIEWS = YES;
GENERATE_INFOPLIST_FILE = YES;
Expand Down Expand Up @@ -1394,7 +1394,7 @@
ASSETCATALOG_COMPILER_GLOBAL_ACCENT_COLOR_NAME = AccentColor;
CODE_SIGN_ENTITLEMENTS = "PomPadDo.watch Watch App/PomPadDo.watch Watch App.entitlements";
CODE_SIGN_STYLE = Automatic;
CURRENT_PROJECT_VERSION = 109;
CURRENT_PROJECT_VERSION = 110;
DEVELOPMENT_ASSET_PATHS = "\"PomPadDo.watch Watch App/Preview Content\"";
ENABLE_PREVIEWS = YES;
GENERATE_INFOPLIST_FILE = YES;
Expand Down Expand Up @@ -1427,7 +1427,7 @@
CLANG_ENABLE_MODULES = YES;
CODE_SIGN_ENTITLEMENTS = PomPadDoWidgetsExtension.entitlements;
CODE_SIGN_STYLE = Automatic;
CURRENT_PROJECT_VERSION = 109;
CURRENT_PROJECT_VERSION = 110;
GENERATE_INFOPLIST_FILE = YES;
INFOPLIST_FILE = PomPadDoWidgets/Info.plist;
INFOPLIST_KEY_CFBundleDisplayName = PomPadDoWidgets;
Expand Down Expand Up @@ -1462,7 +1462,7 @@
CLANG_ENABLE_MODULES = YES;
CODE_SIGN_ENTITLEMENTS = PomPadDoWidgetsExtension.entitlements;
CODE_SIGN_STYLE = Automatic;
CURRENT_PROJECT_VERSION = 109;
CURRENT_PROJECT_VERSION = 110;
GENERATE_INFOPLIST_FILE = YES;
INFOPLIST_FILE = PomPadDoWidgets/Info.plist;
INFOPLIST_KEY_CFBundleDisplayName = PomPadDoWidgets;
Expand Down Expand Up @@ -1496,7 +1496,7 @@
CODE_SIGN_IDENTITY = "Apple Development";
"CODE_SIGN_IDENTITY[sdk=macosx*]" = "Apple Development";
CODE_SIGN_STYLE = Automatic;
CURRENT_PROJECT_VERSION = 109;
CURRENT_PROJECT_VERSION = 110;
ENABLE_APP_SANDBOX = YES;
ENABLE_HARDENED_RUNTIME = YES;
ENABLE_USER_SELECTED_FILES = readonly;
Expand Down Expand Up @@ -1531,7 +1531,7 @@
CODE_SIGN_IDENTITY = "Apple Development";
"CODE_SIGN_IDENTITY[sdk=macosx*]" = "Apple Development";
CODE_SIGN_STYLE = Automatic;
CURRENT_PROJECT_VERSION = 109;
CURRENT_PROJECT_VERSION = 110;
ENABLE_APP_SANDBOX = YES;
ENABLE_HARDENED_RUNTIME = YES;
ENABLE_USER_SELECTED_FILES = readonly;
Expand Down Expand Up @@ -1695,7 +1695,7 @@
CODE_SIGN_ENTITLEMENTS = PomPadDo/PomPadDoMac.entitlements;
CODE_SIGN_STYLE = Automatic;
COMBINE_HIDPI_IMAGES = YES;
CURRENT_PROJECT_VERSION = 109;
CURRENT_PROJECT_VERSION = 110;
DEAD_CODE_STRIPPING = YES;
DEVELOPMENT_ASSET_PATHS = "\"PomPadDo/Preview Content\"";
ENABLE_APP_SANDBOX = YES;
Expand Down Expand Up @@ -1733,7 +1733,7 @@
CODE_SIGN_ENTITLEMENTS = PomPadDo/PomPadDoMac.entitlements;
CODE_SIGN_STYLE = Automatic;
COMBINE_HIDPI_IMAGES = YES;
CURRENT_PROJECT_VERSION = 109;
CURRENT_PROJECT_VERSION = 110;
DEAD_CODE_STRIPPING = YES;
DEVELOPMENT_ASSET_PATHS = "\"PomPadDo/Preview Content\"";
ENABLE_APP_SANDBOX = YES;
Expand Down Expand Up @@ -1767,7 +1767,7 @@
buildSettings = {
BUNDLE_LOADER = "$(TEST_HOST)";
CODE_SIGN_STYLE = Automatic;
CURRENT_PROJECT_VERSION = 109;
CURRENT_PROJECT_VERSION = 110;
DEAD_CODE_STRIPPING = YES;
GENERATE_INFOPLIST_FILE = YES;
MACOSX_DEPLOYMENT_TARGET = 14.0;
Expand All @@ -1785,7 +1785,7 @@
buildSettings = {
BUNDLE_LOADER = "$(TEST_HOST)";
CODE_SIGN_STYLE = Automatic;
CURRENT_PROJECT_VERSION = 109;
CURRENT_PROJECT_VERSION = 110;
DEAD_CODE_STRIPPING = YES;
GENERATE_INFOPLIST_FILE = YES;
MACOSX_DEPLOYMENT_TARGET = 14.0;
Expand All @@ -1802,7 +1802,7 @@
isa = XCBuildConfiguration;
buildSettings = {
CODE_SIGN_STYLE = Automatic;
CURRENT_PROJECT_VERSION = 109;
CURRENT_PROJECT_VERSION = 110;
DEVELOPMENT_TEAM = 9Z68336878;
GENERATE_INFOPLIST_FILE = YES;
MACOSX_DEPLOYMENT_TARGET = 26.0;
Expand All @@ -1822,7 +1822,7 @@
isa = XCBuildConfiguration;
buildSettings = {
CODE_SIGN_STYLE = Automatic;
CURRENT_PROJECT_VERSION = 109;
CURRENT_PROJECT_VERSION = 110;
DEVELOPMENT_TEAM = 9Z68336878;
GENERATE_INFOPLIST_FILE = YES;
MACOSX_DEPLOYMENT_TARGET = 26.0;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Коротко: по сути это механическое повышение 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)), чтобы инкремент действительно попадал в бандлы расширений/виджетов.
  • Согласованность версий между контейнером и расширениями/виджетами:

    • Хотя формально не обязательно, лучше держать одинаковые 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.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Коротко: вы просто подняли 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.
        • Убедиться, что значения везде одинаковы.
  • Пресняк на рассинхрон:
    • В 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-валидацию, чтобы больше не ловить несовпадения версий между бандлами.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Коротко по делу — что стоит поправить и как упростить обслуживание версии:

  • Единый источник правды для 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 решат большинство рисков на будущие релизы.

Expand Down
109 changes: 61 additions & 48 deletions Shared/Activities/Focus/FocusTasksView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -18,20 +18,55 @@ struct FocusTasksView: View {
@Query(filter: TasksQuery.predicateTodayActive()) var tasksTodayActive: [Todo]

var body: some View {
List(selection: $selectedTask) {
ForEach(tasksTodayActive.sorted(by: TasksQuery.defaultSorting),
id: \.self) { task in
if task.hasSubtasks() {
OutlineGroup([task],
id: \.self,
children: \.visibleSubtasks) { maintask in
if tasksTodayActive.count > 0 {
List(selection: $selectedTask) {
ForEach(tasksTodayActive.sorted(by: TasksQuery.defaultSorting),
id: \.self) { task in
if task.visibleSubtasks != nil {
OutlineGroup([task],
id: \.self,
children: \.visibleSubtasks) { maintask in
HStack {
TaskRowView(task: maintask)
.modifier(FocusTaskRowModifier(task: maintask, viewMode: $viewMode))
.tag(maintask)
.listRowSeparator(.hidden)

if let focus = focusTask.task, focus == maintask {
Button {
timer.reset()
if timer.mode == .pause || timer.mode == .longbreak {
timer.skip()
}
focusTask.task = nil
} label: {
Image(systemName: "stop.fill")
}
} else {
Button {
focusTask.task = maintask
viewMode = 1
if timer.state == .idle {
timer.reset()
timer.start()
} else if timer.state == .paused {
timer.resume()
}
} label: {
Image(systemName: "play.fill")
}
.accessibility(identifier: "\(maintask.name)PlayButton")
}
}
}
.listRowSeparator(.hidden)
} else {
HStack {
TaskRowView(task: maintask)
.modifier(FocusTaskRowModifier(task: maintask, viewMode: $viewMode))
.tag(maintask)
.listRowSeparator(.hidden)
TaskRowView(task: task)
.modifier(FocusTaskRowModifier(task: task, viewMode: $viewMode))
.tag(task)

if let focus = focusTask.task, focus == maintask {
if let focus = focusTask.task, focus == task {
Button {
timer.reset()
if timer.mode == .pause || timer.mode == .longbreak {
Expand All @@ -43,7 +78,7 @@ struct FocusTasksView: View {
}
} else {
Button {
focusTask.task = maintask
focusTask.task = task
viewMode = 1
if timer.state == .idle {
timer.reset()
Expand All @@ -54,46 +89,24 @@ struct FocusTasksView: View {
} label: {
Image(systemName: "play.fill")
}
.accessibility(identifier: "\(maintask.name)PlayButton")
}
}
}
.listRowSeparator(.hidden)
} else {
HStack {
TaskRowView(task: task)
.modifier(FocusTaskRowModifier(task: task, viewMode: $viewMode))
.tag(task)

if let focus = focusTask.task, focus == task {
Button {
timer.reset()
if timer.mode == .pause || timer.mode == .longbreak {
timer.skip()
}
focusTask.task = nil
} label: {
Image(systemName: "stop.fill")
.accessibility(identifier: "\(task.name)PlayButton")
}
} else {
Button {
focusTask.task = task
viewMode = 1
if timer.state == .idle {
timer.reset()
timer.start()
} else if timer.state == .paused {
timer.resume()
}
} label: {
Image(systemName: "play.fill")
}
.accessibility(identifier: "\(task.name)PlayButton")
}
.listRowSeparator(.hidden)
}
.listRowSeparator(.hidden)
}
}
} else {
VStack {
Spacer()
Image(systemName: "checkmark.circle")
.resizable()
.foregroundStyle(Color.gray)
.frame(width: 100, height: 100)

Text("No tasks for today")
Spacer()
}
}
}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ниже — только замечания по проблемным/сомнительным местам и предложениям улучшения.

Структура/дублирование

  • Дублирование разметки строк (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: FocusModel

    var 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 у детей).

Эти шаги уберут дубли, стабилизируют идентичности, улучшат доступность и сделают логику таймера предсказуемой.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ниже — только те замечания, которые реально улучшат код или устранят потенциальные проблемы.

  • Стабильные идентификаторы вместо .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() }
  • Сравнение по объектам вместо по 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)
        }
  • Проверка пустоты коллекции

    • if tasksTodayActive.count > 0 замените на более идиоматичное if !tasksTodayActive.isEmpty.
  • Пустое состояние: сохраняйте стабильную структуру

    • Сейчас при пустом списке вы возвращаете VStack вместо List. Это приводит к «прыгающему» layout/стилю и может ломать selection-поведение. Лучше всегда возвращать List, а пустое состояние показывать через .overlay или ContentUnavailableView (iOS 17+):
      • List(selection: $selectedTask) { ... }
        .overlay {
        if tasksTodayActive.isEmpty { EmptyStateView() }
        }
  • Сортировка в @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
  • Доступность и тестируемость

    • Идентификатор для stop-кнопки отсутствует, для play — есть. Синхронизируйте: добавьте .accessibilityIdentifier и для Stop. Также используйте стабильные ID в идентификаторах (например, persistentModelID.uuidString), а не имя задачи:
      • .accessibilityIdentifier("task-(task.persistentModelID)-play")
      • .accessibilityIdentifier("task-(task.persistentModelID)-stop")
    • Параллельно добавьте .accessibilityLabel и .accessibilityHint — это улучшит UX с VoiceOver.
  • Цвета в empty state

    • .foregroundStyle(Color.gray) замените на .foregroundStyle(.secondary), чтобы корректно адаптироваться к темам/доступности.
  • Кнопки внутри List

    • Если таргетите macOS/iPadOS c selection в List, стоит явно указать стиль, чтобы нажатие на кнопки не конфликтовало с выбором строки:
      • .buttonStyle(.borderless)
  • Семантика hasSubtasks() → visibleSubtasks?.isEmpty == false

    • Вы изменили критерий с «есть подзадачи» на «есть видимые подзадачи». Если фильтрация «невидимых» — осознанное требование, ок. Если нет — есть риск скрыть родителя с невидимыми подзадачами и изменить UX разворачивания. Уточните, что именно хотите показать.

Итоговая «быстрая выжимка» того, что стоит сделать в первую очередь:

  • Перейти на стабильные ID в ForEach/OutlineGroup/.tag и сравнениях.
  • Вынести Play/Stop в переиспользуемый компонент и поправить логику reset/skip.
  • Держать всегда List и показывать пустое состояние через overlay.
  • Перенести сортировку в @query.
  • Убрать .listRowSeparator с дочерних сабвью и повесить на корневой HStack строки.
  • Заменить viewMode = 1 на enum и поправить a11y-идентификаторы.

Expand Down
2 changes: 1 addition & 1 deletion Shared/Activities/Projects/Board/BoardView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ struct BoardView: View {
.filter({ $0.status == status && $0.parentTask == nil })
.sorted(by: TasksQuery.sortingWithCompleted),
id: \.self) { task in
if task.hasSubtasks() {
if task.visibleSubtasks != nil {
OutlineGroup([task],
id: \.self,
children: \.visibleSubtasks) { maintask in
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ниже — только по делу, по изменённой строке и ближайшему контексту.

  • Семантика условия. Заменив 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.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Наблюдения по изменению 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 уже отсортированы нужным образом.

Expand Down
4 changes: 2 additions & 2 deletions Shared/Activities/Projects/List/ProjectTasksListView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ struct ProjectTasksListView: View {
.filter({ $0.status == status && $0.parentTask == nil })
.sorted(by: TasksQuery.sortingWithCompleted),
id: \.self) { task in
if task.hasSubtasks() {
if task.visibleSubtasks != nil {
OutlineGroup([task],
id: \.self,
children: \.visibleSubtasks) { maintask in
Expand Down Expand Up @@ -150,7 +150,7 @@ struct ProjectTasksListView: View {
)) {
ForEach(section == .completed ? searchResults.filter({ $0.completed && $0.parentTask == nil }) : searchResults.filter({ $0.completed == false }),
id: \.self) { task in
if task.hasSubtasks() {
if task.visibleSubtasks != nil {
OutlineGroup([task],
id: \.self,
children: \.visibleSubtasks) { maintask in
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ниже — только по делу, по текущему изменению и окружению:

  • Смена условия с 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.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ниже — только те замечания, которые действительно могут улучшить код или предотвратить проблемы.

  • Семантика изменения 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.

Если кратко, основные практические шаги:

  1. Перейти на стабильные идентификаторы (id: .id).
  2. Заменить условие на hasVisibleSubtasks или явную распаковку.
  3. Рассмотреть один OutlineGroup на секцию вместо OutlineGroup на каждый элемент.
  4. Убедиться в согласованной сортировке children и намеренности смены семантики «видимость vs наличие» подзадач.

Expand Down
2 changes: 1 addition & 1 deletion Shared/Activities/Projects/ProjectsListView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -222,8 +222,8 @@ struct ProjectsListView: View {
Button {
for project in projects.filter({ $0.group == group }) {
project.group = nil
modelContext.delete(group)
}
modelContext.delete(group)
} label: {
Image(systemName: "trash")
.foregroundStyle(Color.red)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ниже — только содержательные замечания по сути изменения.

  • Семантика удаления: перенести 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), чтобы упростить тестирование и переиспользование.

Expand Down
2 changes: 1 addition & 1 deletion Shared/Activities/Tasks/Subtasks/SubtasksListView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ struct SubtasksListView: View {
)) {
ForEach(section == .completed ? searchResults.filter({ $0.completed && ($0.parentTask == nil || $0.parentTask == mainTask) }) : searchResults.filter({ $0.completed == false }),
id: \.self) { task in
if task.hasSubtasks() {
if task.visibleSubtasks != nil {
OutlineGroup([task],
id: \.self,
children: \.visibleSubtasks) { maintask in
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ниже только то, что действительно может улучшить код и предотвратить регрессии.

  • Условие 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 вели себя согласованно.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Наблюдения по изменению и коду вокруг него:

  • Семантическая разница: переход с 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 и выравнивание логики фильтра по секциям.

Expand Down
2 changes: 1 addition & 1 deletion Shared/Activities/Tasks/TasksListView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ struct TasksListView: View {
)) {
ForEach(section == .completed ? searchResults.filter({ $0.completed && ($0.parentTask == nil) }) : searchResults.filter({ $0.completed == false }),
id: \.self) { task in
if task.hasSubtasks() {
if task.visibleSubtasks != nil {
OutlineGroup([task],
id: \.self,
children: \.visibleSubtasks) { maintask in
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ниже — только предметные замечания по изменению 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.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ниже — только те замечания, которые реально повлияют на корректность, читаемость и поведение.

  • Изменение семантики: переход с 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)
  • Чтение optional в условии:

    • if task.visibleSubtasks?.isEmpty == false хуже читается и даёт шанс на ошибки. Предпочтительно:
      if let subtasks = task.visibleSubtasks, !subtasks.isEmpty { ... }
      или
      if !(task.visibleSubtasks?.isEmpty ?? true) { ... }
  • Производительность visibleSubtasks:

    • Если visibleSubtasks — дорогой фильтр (особенно зависящий от поисковой строки), кэшируйте/готовьте его в модели, чтобы не пересчитывать на каждый ререндер.

Итоговая форма секции с учётом правок может выглядеть так:

  • заранее посчитать roots по разделу (только parentTask == nil);
  • один OutlineGroup с children: .visibleSubtasks;
  • стабильные id;
  • без дополнительных if перед OutlineGroup.

Expand Down
16 changes: 16 additions & 0 deletions Shared/Localizable.xcstrings
Original file line number Diff line number Diff line change
Expand Up @@ -20409,6 +20409,22 @@
}
}
},
"No tasks for today" : {
"localizations" : {
"en" : {
"stringUnit" : {
"state" : "translated",
"value" : "No tasks for today"
}
},
"ru" : {
"stringUnit" : {
"state" : "translated",
"value" : "Нет задач на сегодня"
}
}
}
},
"None" : {
"localizations" : {
"ar" : {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ниже только предметные замечания и улучшения по добавленной строке в .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 для количества задач — лучше интегрировать нулевую форму туда и не добавлять отдельный ключ.

Expand Down
Loading