Skip to content

Conversation

@NullIsOne
Copy link
Collaborator

@NullIsOne NullIsOne commented Jul 19, 2023

Что сделано?

  • Добавлено StackView по стандартам библиотеки компонентов
  • Усовершенствован StackCellGenerator для построения вьюшек из Nib
  • Проставлен intrisinctContentSize для отображения TitleTableViewCell внутри стека
  • Добавлен пример использования нового StackView внутри коллекции и таблицы
  • Добавлена возможность декорировать инстанс базового генератора
  • Добавлены трансформации в Foldable и Diffable
  • BaseCellGenerator и DiffableCellGenerator теперь привязаны одновременно к UITableViewCell и UICollectionCell. Дубли коллекции удалены.
  • FoldableItem и FoldableCellGenerator теперь универсальные и используют generic extensions для связки с таблицей/коллекцией
  • Добавлена возможность декорировать BaseCellGenerator и трансформировать в Foldable или Diffable
  • ...

Зачем это сделано?

Чтобы уменьшить количество сопровождающих классов при создании сложных ячеек на основе сочетания стека и уже готовых ячеек

На что обратить внимание?

  • ПР драфтовый для раннего обсуждения
  • TBD разрешить todo в TableWrappedCell и CollectionWrappedCell
    Добавлен EmptyViewGenerator для генерации View, но без установки Model
    Установка модели будет произведена в ViewWrapper.
  • TBD Merge 7.4 and resolve conflicts #254 (comment) поправить расчет стека при внедрении ячейки внутрь стека (нетиповое использование) исправлено добавлением intrinsicContentSize ячейке
  • TBD generic функция generator для контекстно-зависимого вызова и возможного отбрасывания SomeView.rddm. для упрощения синтаксиса добавлены Context для упрощения синтакциса
  • TableStack, CollectionStack и пр. пока не удалены , но будут после улучшения синтаксиса билдера. Добавлено все неободимое чтобы приблизить синтаксис к эталонному желаемому варианту.
  • Использование ячейки внутри стека нецелевое, но нужно для показа возможности
  • Напоминаю, что mutating и Property спрячутся за макрос, который ожидает релиза Xcode 15
  • Надо придумать как ограничить трансформации генераторов ведь синтаксис позволяет сделать asFoldable.asDiffable но последняя трансформация в текущей реализации "перекроет" первую..
  • ...

Как протестировать?

  • Тапнуть в примере на строчку "Table with stack cell"

@github-actions
Copy link

github-actions bot commented Jul 19, 2023

Warnings
⚠️ Oops! We have found some issues. It's better to fix them to keep code clean

SwiftLint found issues

Severity File Reason
Warning BakgroundStyle.swift:32 Shorthand syntactic sugar should be used, i.e. Int? instead of Optional. (syntactic_sugar)
Warning BorderStyle.swift:42 Shorthand syntactic sugar should be used, i.e. Int? instead of Optional. (syntactic_sugar)
Error ComponentsOverviewTableViewController.swift:26 Line should be 145 characters or less: currently 589 characters (line_length)

Generated by 🚫 Danger Swift against b75c607

@NullIsOne NullIsOne force-pushed the feature/SPT-1555/universal_stack branch from b4f619b to 40be511 Compare July 19, 2023 09:24
@codecov-commenter
Copy link

codecov-commenter commented Jul 19, 2023

Codecov Report

❌ Patch coverage is 68.07018% with 91 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.22%. Comparing base (cbf811a) to head (b75c607).
⚠️ Report is 8 commits behind head on develop.

Files with missing lines Patch % Lines
Source/Common/Generators/BaseCellGenerator.swift 71.25% 23 Missing ⚠️
...urce/Common/Generators/FoldableCellGenerator.swift 74.19% 16 Missing ⚠️
...rce/Collection/Wrapper/CollectionWrappedCell.swift 7.69% 12 Missing ⚠️
Source/Table/Wrapper/TableWrappedCell.swift 7.69% 12 Missing ⚠️
Source/Table/Wrapper/TableWrappedCell+RDDM.swift 27.27% 8 Missing ⚠️
Source/Table/Protocols/TableChildrenHolder.swift 0.00% 5 Missing ⚠️
Source/Stack/Generators/BaseViewGenerator.swift 89.74% 3 Missing and 1 partial ⚠️
Source/Models/Section.swift 0.00% 3 Missing ⚠️
...ble/Plugins/PluginAction/TableFoldablePlugin.swift 72.72% 3 Missing ⚠️
...lugins/PluginAction/CollectionFoldablePlugin.swift 60.00% 2 Missing ⚠️
... and 2 more
Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #253      +/-   ##
===========================================
- Coverage    72.66%   72.22%   -0.44%     
===========================================
  Files          161      166       +5     
  Lines         4905     5113     +208     
  Branches      2224     2295      +71     
===========================================
+ Hits          3564     3693     +129     
- Misses        1221     1299      +78     
- Partials       120      121       +1     
Flag Coverage Δ
uitests 64.03% <66.66%> (-0.06%) ⬇️
unittests 32.64% <13.33%> (-1.39%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

])
},
and: .class)
])
Copy link
Contributor

Choose a reason for hiding this comment

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

Все это дело не читается, пытался добавить в коллекцию, сам не понял, пришлось копипастить

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ясное дело. Упрощение синтаксиса в работе. (см "На что обратить внимание")
Референсный вариант специально временно сохранен чтобы на него можно было опираться.

Copy link
Contributor

@kombatkos kombatkos left a comment

Choose a reason for hiding this comment

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

Настроить так чтобы стек принимал не генераторы, а ConfigurableItem вью.
Упростить реализацию иначе никто пользоваться не будет.
В идеале как в swiftUI HStack(space: 8) { ConfigurableItem }

Если оставить старую реализацию, то можно убрать сабклассы и оставить такое
TableStack(axis: .horizontal) {
ConfigurableItemView
}


// MARK: - Public properties

private(set) public var children: [StackCellGenerator] = []
Copy link
Contributor

Choose a reason for hiding this comment

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

должен принимать any ConfigurableItem, идея заключалась в том чтобы на проектах собирать ячейки из ячеек или вьюх. С таким подходом это только будут вью

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Категорически не согласен.
Ячейка тоже UIView. Ей доступен вызов rddm.baseStackCellGenerator метода.
В примере показано

Copy link
Contributor

Choose a reason for hiding this comment

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

Но он не принимает такое TextCell.buildView("Some text")

Copy link
Contributor

Choose a reason for hiding this comment

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

Что мешает принимать ConfigurableItem?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

.buildView производит View. При этом он грузит ее из Nib, если указано.
Это ресурсоемкая задача.

Основа rddm в генераторах, которые инициализируются моделью и производят View по запросу внутри адаптера.

Передача View объектов меняет порядок наполнения адаптера и стоимость (по ресурсам) этого процесса.
Такой шаг также переворачивает метод использования библиотеки. Генераторы уходят на задний план. Это неправильно.


// MARK: - Model

public struct Model: AlignmentProvider {
Copy link
Contributor

Choose a reason for hiding this comment

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

можно вынести отдельно в extension StackView

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Как и говорил вся модель будет под макросом.
Опиши проперти и гуляй.

@NullIsOne
Copy link
Collaborator Author

Обновил синтаксис. По-моему уже лучше, но не идеал конечно.

2 проблемы, с которыми столкнулся

  1. resultBuilder не позволяет использовать статические инициализаторы
    Решено также как в Editor.Property классе передачей типа: равно точки входа на фабрику (ViewContext, TableContext, CollectionContext).
    Контексты можно расширять своими функциями и шорткатами.
  2. generic функции надо конкретизировать
    Пришлось в generic функцию добавить параметр для конкретизации.
    it.viewNib(type: TitleTableViewCell.self, model: "1")
    против
    TitleTableViewCell.rddm.viewGenerator(with: "1", and: .nib)
    Надо оценить насколько это интуитивно.

Таким образом контекст определяет для какой коллекции нужен генератор.
Генераторы создаются через контекст.

В случае со стеком вернул в примере установку children вместо выноса в отдельный параметр, чтобы исключить неочевидные ошибки и конфликты при построении стека и для более древовидной структуры.

@NullIsOne NullIsOne force-pushed the feature/SPT-1555/universal_stack branch from 3ba1542 to a5ba3d7 Compare July 21, 2023 08:38
@NullIsOne NullIsOne force-pushed the feature/SPT-1555/universal_stack branch from a5ba3d7 to 4debacb Compare July 25, 2023 10:34
@NullIsOne NullIsOne force-pushed the feature/SPT-1555/universal_stack branch from d7c7f04 to bb18efd Compare July 26, 2023 15:01
@NullIsOne
Copy link
Collaborator Author

NullIsOne commented Jul 26, 2023

Очередное обновление и сразу 2 варианта синтаксиса.

  1. it.gen(CellOrViewType.self, model: Model)
    Снимок экрана 2023-07-26 в 18 19 09
    Полностью рабочий, но читаемость все еще далека.
  2. CellOrViewType.build(in: Context, with: Model)
    Снимок экрана 2023-07-26 в 18 31 48
    По сути extension к предыдущему решению.
    Есть проблема с TableCell классами. Они не выходят на создание baseGenerator из-за чего получаем краш TableWrappedCell<SomeOtherCell> вместо генератора для SomeOtherCell.

Пока склоняюсь ко второму варианту, но его еще надо починить.

Кстати, оба варианта (плюс предыдущий) заваливают варнингами типа такого
Снимок экрана 2023-07-26 в 18 18 40
Это напоминает о том, что если уж надо какую-то верстку из ячейки запинуть в стек, то лучше отрефакторить, вынеся необходимый кусок во вью.
Отобразить ячейку возможно, но могут быть проблемы с ресайзом, что впринципе и показывает a11y аудит.

@NullIsOne NullIsOne force-pushed the feature/SPT-1555/universal_stack branch from bb18efd to 8779dbc Compare July 26, 2023 15:14
@NullIsOne NullIsOne force-pushed the feature/SPT-1555/universal_stack branch from 33e299e to a4a499d Compare July 27, 2023 11:03
@NullIsOne NullIsOne force-pushed the feature/SPT-1555/universal_stack branch from 423bc1c to c9ad719 Compare July 27, 2023 12:40
@NullIsOne NullIsOne force-pushed the feature/SPT-1555/universal_stack branch from 5ead114 to df6434e Compare July 28, 2023 13:14
@NullIsOne NullIsOne added the Blocker Blocked completion all other issues label Jul 31, 2023
@NullIsOne NullIsOne marked this pull request as ready for review July 31, 2023 09:14
@NullIsOne
Copy link
Collaborator Author

NullIsOne commented Jul 31, 2023

Убрал ready for review и поставил blocker.

Изменений много. Их надо влить чтобы не мучаться потом с конфликтами.

Синтаксис оригинала достигнут.
Добавлена возможность декорировать и трансформировать генераторы.

Тесты на Xcode 15 с performAccessibilityAudit успешны.
Снимок экрана 2023-08-01 в 12 05 00

@NullIsOne NullIsOne force-pushed the feature/SPT-1555/universal_stack branch from b9ef96d to 7bfcfca Compare July 31, 2023 11:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

8.0 Blocker Blocked completion all other issues enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants