surfstudio / ReactiveDataDisplayManager

MIT License
34 stars 13 forks source link

[SPT-1478] Интерактивность MessageView #244

Closed Golubeykov closed 1 year ago

Golubeykov commented 1 year ago

SPT-1478

Что сделано?

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

Добавить новые возможности для библиотеки. Пример - взаимодействие с ячейкой сообщения в чате.

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

1) Обнаружил баг, который так же есть в базовой ветке и аффектит эту задачу. На момент создания ПРа пофиксить не получилось. Суть заключается в том, что при инициализации таблицы поверх основного contentView ячейки поверх создается еще один пустой contentView. Он перехватывает на себя нажатия. Но если пролистать таблицу ниже, при переиспользовании, этот пустой contentView пропадает. Пробовал убирать другие элементы по типу дат и спейсера, крутил-вертел генераторы. Разобраться не получилось. Вот скрин с пустой contentView над ячейкой (выделена синим).

odd content view

2) Если добавлять на MessageView одновременно tapHandler и dataDetectionHandler, то обрабатываться будет только tapHandler (т.к. один перекрывает другой). Пока что не смотрел, как можно их совместить. Дайте знать, если требуется совместить.

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

Зайти в Table -> Table with custom components -> Проскроллить полученные сообщения (тап работать не будет из-за бага выше) -> Дойдя до отправленных сообщений, нажать на ссылку, попробовать выделить/скопировать текст -> Просколллить вверх, теперь на полученные сообщения можно нажимать, должен появиться алерт

Демо https://github.com/surfstudio/ReactiveDataDisplayManager/assets/47087482/3e437d2b-839a-4d44-a138-edbbc4864b61
github-actions[bot] commented 1 year ago
Warnings
:warning: Oops! We have found some issues. It's better to fix them to keep code clean

SwiftLint found issues

Severity File Reason
Warning DataDetectionStyle.swift:24 Line should be 145 characters or less: currently 153 characters (line_length)
Warning MessageView.swift:137 Redundant explicit declaration of default types should be avoided (redundant_default_type)
Warning MessageView.swift:267 Line should be 145 characters or less: currently 151 characters (line_length)
Warning MessageView.swift:115 Lines should not have trailing whitespace. (trailing_whitespace)
Warning ComponentsOverviewTableViewController.swift:109 Line should be 145 characters or less: currently 148 characters (line_length)

Generated by :no_entry_sign: Danger Swift against 21ab6012688521bbe8f62dac77762ba67db563fa

codecov-commenter commented 1 year ago

Codecov Report

Patch coverage has no change and project coverage change: +1.94 :tada:

Comparison is base (9c1e698) 71.06% compared to head (21ab601) 73.00%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## develop #244 +/- ## =========================================== + Coverage 71.06% 73.00% +1.94% =========================================== Files 146 146 Lines 4472 4472 Branches 2007 2007 =========================================== + Hits 3178 3265 +87 + Misses 1194 1099 -95 - Partials 100 108 +8 ``` | Flag | Coverage Δ | | |---|---|---| | uitests | `62.99% <ø> (+1.94%)` | :arrow_up: | | unittests | `37.11% <ø> (ø)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=surfstudio#carryforward-flags-in-the-pull-request-comment) to find out more. [see 8 files with indirect coverage changes](https://app.codecov.io/gh/surfstudio/ReactiveDataDisplayManager/pull/244/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=surfstudio)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

NullIsOne commented 1 year ago

@Golubeykov проблема с несрабатыванием тапа и невидимой contentView была в том, что наш ViewWrapper добавлял MessageView непосредственно к ячейке, а не к contentView. reuse судя по всему фиксил это и тапы начинали работать.

Я также изменил способ подсветки сообщения, чтобы на стейты опираться, а не на анимацию дополнительную, но так сразу не взлетело..) Еще подумаю как улучшить.