Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SPT-1555] Universal StackView #253

Open
wants to merge 30 commits into
base: develop
Choose a base branch
from

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: 13.33% and project coverage change: -40.02% ⚠️

Comparison is base (cbf811a) 72.66% compared to head (ea60f8c) 32.64%.

Additional details and impacted files
@@             Coverage Diff              @@
##           develop     #253       +/-   ##
============================================
- Coverage    72.66%   32.64%   -40.02%     
============================================
  Files          161      166        +5     
  Lines         4905     5113      +208     
  Branches      2224     2285       +61     
============================================
- Hits          3564     1669     -1895     
- Misses        1221     3378     +2157     
+ Partials       120       66       -54     
Flag Coverage Δ
uitests ?
unittests 32.64% <13.33%> (-1.39%) ⬇️

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

Files Changed Coverage Δ
Source/Collection/CollectionCell+RDDM.swift 0.00% <0.00%> (-100.00%) ⬇️
Source/Collection/CollectionSection.swift 0.00% <0.00%> (-100.00%) ⬇️
...rs/CalculatableHeightCollectionCellGenerator.swift 0.00% <0.00%> (-100.00%) ⬇️
...ors/CalculatableWidthCollectionCellGenerator.swift 0.00% <0.00%> (ø)
...urce/Collection/Generators/CollectionContext.swift 0.00% <0.00%> (ø)
...lugins/PluginAction/CollectionFoldablePlugin.swift 6.97% <0.00%> (-74.42%) ⬇️
...ollection/Protocols/CollectionChildrenHolder.swift 0.00% <0.00%> (ø)
...ollection/Wrapper/CollectionWrappedCell+RDDM.swift 0.00% <0.00%> (ø)
...rce/Collection/Wrapper/CollectionWrappedCell.swift 0.00% <0.00%> (ø)
...urce/Common/Generators/DiffableCellGenerator.swift 0.00% <0.00%> (ø)
... and 20 more

... and 83 files with indirect coverage changes

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

])
},
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 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.

None yet

3 participants