IOS-7874 make fix space for title header #3970

skibinalexander · 2024-09-29T15:29:40Z

В рамках изменения дизайна, дизайнеры заменили по своим компонентам отступы и попросили привести все порядок.

На данный компонент использовался на трех экранах насколько я понимаю:

Organize Tokens
Markets Token Detail
Markets Token Selector

@Andoran90 вероятно по МР будем обсуждать, но лейоут насколько вижу точно был не совсем такой, как требует дизайн, поэтому пришлось вносить правки

Есть специфай хедеры которые не легли в CommonHeader

…_header

Andoran90

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

Andoran90 · 2024-09-30T10:42:28Z

.../Modules/Markets/TokenMarketsDetails/Subviews/Insights/MarketsTokenDetailsInsightsView.swift

        }
-        .defaultRoundedBackground(with: Colors.Background.action, horizontalPadding: Constants.backgroundHorizontalPadding)
+        .defaultRoundedBackground(with: Colors.Background.action, verticalPadding: .zero, horizontalPadding: Constants.backgroundHorizontalPadding)


какой смысл этих правок? vertiticalPadding = 12, тут удаляется паддинг, добавляется в заголовок сверху отдельный паддинг, отдельный в LazyVGrid... по коду не вижу вообще никакого смысла этих правок

какой смысл этих правок? vertiticalPadding = 12, тут удаляется паддинг, добавляется в заголовок сверху отдельный паддинг, отдельный в LazyVGrid... по коду не вижу вообще никакого смысла этих правок

Если посмотреть отступы в фигме, то сейчас они соответствуют отступам дизайна

Andoran90 · 2024-09-30T10:43:01Z

.../Modules/Markets/TokenMarketsDetails/Subviews/Insights/MarketsTokenDetailsInsightsView.swift

    }

    private var header: some View {
-        HStack {
+        HStack(alignment: .center) {


по умолчанию - .center, зачем это добавлять?

по умолчанию - .center, зачем это добавлять?

Возможно лишнее , но вообще много тоже где пишем, уберу

Andoran90 · 2024-09-30T10:48:36Z

Tangem/UIComponents/CommonHeaders/CommonHeaderTitleButtonView.swift

+    private(set) var title: String
+    private(set) var button: ButtonInput
+    private(set) var action: (() -> Void)?


зачем тут private(set)? они же не могут изменяться изнутри. это все let не приватные должны быть

зачем тут private(set)? они же не могут изменяться изнутри. это все let не приватные должны быть

Хотел заприватить, оставлю let, просто по последним МР часто стали писать так

ну так норм писать во вью моделях, но не во вьюхах, во вьюхах вообще смысла нет, это надо чтобы функции были mutating и изнутри менять что-то, вьюха почти всегда просто показывает что ей дали

ну так норм писать во вью моделях, но не во вьюхах, во вьюхах вообще смысла нет, это надо чтобы функции были mutating и изнутри менять что-то, вьюха почти всегда просто показывает что ей дали

Да, согласен, убрал

Andoran90 · 2024-09-30T10:49:30Z

Tangem/UIComponents/CommonHeaders/CommonHeaderTitleButtonView.swift

+
+/// This is a common component of the system design - a common header + button.
+/// Required due to specific dimensions
+struct CommonHeaderTitleButtonView: View {


вообще не понимаю смысла этой вьюхи, она используется в одном месте

вообще не понимаю смысла этой вьюхи, она используется в одном месте

Ответ в целом простой, это компонент DS со специфичными отступами, сейчас юзается в одном месте, но по сути это компонент же

Andoran90 · 2024-09-30T10:50:19Z

Tangem/UIComponents/CommonHeaders/CommonHeaderTitleButtonView.swift

+        @Binding var isDisabled: Bool
+        @Binding var isLoading: Bool


биндинги нужны только в случае двухсторонней связи - когда изнутри значение может меняться, а тут оно не меняется, а просто передается. Поэтому let должно быть достаточно.

биндинги нужны только в случае двухсторонней связи - когда изнутри значение может меняться, а тут оно не меняется, а просто передается. Поэтому let должно быть достаточно.

Да, согласен оверхед

Andoran90 · 2024-09-30T11:35:47Z

Tangem/UIComponents/CommonHeaders/CommonHeaderTitleView.swift

+/// This is a common component of the system design - a common header + button.
+/// Required due to specific dimensions
+struct CommonHeaderTitleView: View {
+    private(set) var title: String


private(set) var тут не нужен.

Вообще не выглядит как нужная вьюха, т.к. она не так много где используется, в инайтах там есть действие на заголовке, получается оно используется в двух местах и из-за этого сейчас переделывается верстка, чтобы подогнать другие отступы. Не понимаю смысла всех этих правок. Пока что выглядит что из-за правок в дизайне можно обновить точечно отступы.

private(set) var тут не нужен.

Вообще не выглядит как нужная вьюха, т.к. она не так много где используется, в инайтах там есть действие на заголовке, получается оно используется в двух местах и из-за этого сейчас переделывается верстка, чтобы подогнать другие отступы. Не понимаю смысла всех этих правок. Пока что выглядит что из-за правок в дизайне можно обновить точечно отступы.

Ну ответ тут тоже простой, есть компонент, дизайнеры сделали специфичными отступы, полагая что это компонент из фирмы, использование его будет расти, ну и как раз получается, чтобы не выставлять отступы по месту везде, проставить в одном месте и забыть про них

…_header

skibinalexander · 2024-10-01T08:44:20Z

RPReplay_Final1727771772.MP4

Andoran90 · 2024-10-01T11:05:13Z

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

skibinalexander · 2024-10-01T11:19:14Z

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

Сопоставлял, ага, приложу скрин наложения

…_header

Andoran90 · 2024-10-02T07:08:51Z

.../TokenMarketsDetails/Subviews/PricePerformance/MarketsTokenDetailsPricePerformanceView.swift

+            HStack {
+                Text(Localization.marketsTokenDetailsLow)
+                    .style(Fonts.Regular.footnote, color: Colors.Text.tertiary)

-                ProgressView(value: viewModel.pricePerformanceProgress)
-                    .progressViewStyle(TangemProgressViewStyle(
-                        height: 6,
-                        backgroundColor: Colors.Background.tertiary,
-                        progressColor: Colors.Text.accent
-                    ))
-                    .animation(.default, value: viewModel.pricePerformanceProgress)
+                Spacer(minLength: 8)

-                HStack {
-                    Text(viewModel.lowValue)
-                        .style(Fonts.Regular.callout, color: Colors.Text.primary1)
-                        .animation(.default, value: viewModel.lowValue)
+                Text(Localization.marketsTokenDetailsHigh)
+                    .style(Fonts.Regular.footnote, color: Colors.Text.tertiary)
+            }


если делать компонент и отдельно в него выносить, то посмотри как сделан NavigationBar и добавь, пожалуйста, тоже TrailingItem для заголовка, чтобы у нас не было куча вариантов. Можно сделать инициализатор, который будет принимать @ViewBuilder для доп.вьюхи и не надо будет везде копипастить код для заголовка и будет единообразно сделано

NavigationBar

Да окей посмотрю, попробую переделать, просто вьюха с количеством настроек как у костюма астронавта тоже на мой взгляд странное решение, в DS явно обрисовали 3 вида компонента схожего по смыслу, зачем нам стараться объединить в один, чтобы избежать копиписасты в Text("") чтобы не смущала только копипаста, на мой взгляд тоже так себе, потому как, чтобы внести изменения , надо весь дизайн где юзается вьюха перетестировать, пример как раз шторка с описанием для маркетсов и стейкинга

skibinalexander · 2024-10-02T07:13:28Z

skibinalexander · 2024-10-02T14:33:33Z

Tangem/UIComponents/CommonHeaders/BlockHeaderTitleButtonView.swift

+
+/// This is a common component of the system design - a common header + button.
+/// Required due to specific dimensions
+struct BlockHeaderTitleButtonView: View {


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

tureck1y

Внимательно не смотрел, проверь плиз наложением макета

skibinalexander added 3 commits September 29, 2024 16:43

IOS-7874 Add common header vuew

df353c8

IOS-7874 Make correct spacing for component

02070ce

Merge branch 'develop' into feature/IOS-7874_make_fix_space_for_title…

5eacc60

…_header

skibinalexander requested a review from Andoran90 September 29, 2024 15:29

skibinalexander requested review from tureck1y and a team as code owners September 29, 2024 15:29

skibinalexander requested a review from fedorov-d September 29, 2024 15:29

Andoran90 reviewed Sep 30, 2024

View reviewed changes

skibinalexander added 2 commits October 1, 2024 15:51

Merge branch 'develop' into feature/IOS-7874_make_fix_space_for_title…

a763f83

…_header

IOS-7874 Fix review comments

b6ceef4

skibinalexander requested a review from Andoran90 October 1, 2024 09:35

Merge branch 'develop' into feature/IOS-7874_make_fix_space_for_title…

41741aa

…_header

Andoran90 reviewed Oct 2, 2024

View reviewed changes

IOS-7874 Fix review comments

75d052b

skibinalexander commented Oct 2, 2024

View reviewed changes

tureck1y approved these changes Oct 2, 2024

View reviewed changes

skibinalexander requested a review from Andoran90 October 3, 2024 07:08

Andoran90 approved these changes Oct 3, 2024

View reviewed changes

fedorov-d approved these changes Oct 3, 2024

View reviewed changes

skibinalexander merged commit 0c821f7 into develop Oct 3, 2024
3 checks passed

skibinalexander deleted the feature/IOS-7874_make_fix_space_for_title_header branch October 3, 2024 09:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

IOS-7874 make fix space for title header #3970

IOS-7874 make fix space for title header #3970

skibinalexander commented Sep 29, 2024

Andoran90 left a comment

Andoran90 Sep 30, 2024

skibinalexander Sep 30, 2024

Andoran90 Sep 30, 2024

skibinalexander Sep 30, 2024

Andoran90 Sep 30, 2024

skibinalexander Sep 30, 2024

Andoran90 Oct 1, 2024

skibinalexander Oct 1, 2024

Andoran90 Sep 30, 2024

skibinalexander Sep 30, 2024

Andoran90 Sep 30, 2024

skibinalexander Sep 30, 2024

Andoran90 Sep 30, 2024

skibinalexander Sep 30, 2024

skibinalexander commented Oct 1, 2024

Andoran90 commented Oct 1, 2024

skibinalexander commented Oct 1, 2024

Andoran90 Oct 2, 2024

skibinalexander Oct 2, 2024 •

edited

Loading

skibinalexander commented Oct 2, 2024

skibinalexander Oct 2, 2024

tureck1y left a comment

IOS-7874 make fix space for title header #3970

IOS-7874 make fix space for title header #3970

Conversation

skibinalexander commented Sep 29, 2024

Andoran90 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

skibinalexander commented Oct 1, 2024

Andoran90 commented Oct 1, 2024

skibinalexander commented Oct 1, 2024

Choose a reason for hiding this comment

skibinalexander Oct 2, 2024 • edited Loading

Choose a reason for hiding this comment

skibinalexander commented Oct 2, 2024

Choose a reason for hiding this comment

tureck1y left a comment

Choose a reason for hiding this comment

skibinalexander Oct 2, 2024 •

edited

Loading