IOS-8236 Token selector view and buy from action buttons #4161

Vyachesl0ve · 2024-11-04T16:22:13Z

ScreenRecording_11-06-2024.01-30-48_1.mov

Vyachesl0ve · 2024-11-04T17:42:01Z

Tangem/Modules/Main/ActionButtons/Buy/BuyTokenSelectorStrings.swift

+
+struct BuyTokenSelectorStrings: TokenSelectorLocalizable {
+    let availableTokensListTitle = Localization.exchangeTokensAvailableTokensHeader
+    let unavailableTokensListTitle = "Unavailable to purchase"


Попросил скинуть текстовочки, в комите с фиксом по ревью долью

Balashov152 · 2024-11-05T09:28:38Z

Tangem/Common/Extensions/Array+.swift

+    var isNotEmpty: Bool {
+        return !isEmpty
+    }


Помню на предыдущей работе обсуждали что "оверхэд"
Хз, можно и оставить, а можно и просто !isEmpty писать

сделал из соображений, что более явно читается, чем !isEmpty

Tangem/Modules/Main/ActionButtons/Buy/Coordinator/ActionButtonsBuyCoordinator.swift

Tangem/Modules/Main/ActionButtons/Buy/View/ActionButtonsBuyCoordinatorView.swift

Tangem/Modules/Main/ActionButtons/Models/ActionButtonsTokenSelectorItem.swift

Tangem/Modules/Main/ActionButtons/ViewModel/ActionButtonsViewModel.swift

Tangem/Modules/Main/MultiWalletMainContent/MultiWalletMainContentViewModel.swift

Tangem/UIComponents/TokenSelectorView/TokenSelectorViewModel.swift

Balashov152 · 2024-11-05T14:40:50Z

Tangem/Modules/Main/ActionButtons/ViewModel/ActionButtonsViewModel.swift


 final class ActionButtonsViewModel: ObservableObject {
+    @Injected(\.exchangeService) private var exchangeService: ExchangeService


Я так понимаю ExpressAvailabilityProvider потом уже будет?)

Balashov152 · 2024-11-05T14:45:36Z

Tangem/Modules/Main/MainCoordinator.swift

@@ -22,6 +22,7 @@ class MainCoordinator: CoordinatorObject {
    @Injected(\.safariManager) private var safariManager: SafariManager
    @Injected(\.pushNotificationsInteractor) private var pushNotificationsInteractor: PushNotificationsInteractor
    @Injected(\.mainBottomSheetUIManager) private var mainBottomSheetUIManager: MainBottomSheetUIManager
+    @Injected(\.tangemApiService) private var tangemApiService: TangemApiService


за это точно наругают, тут появляется логика в coordinator, так нельзя )

Balashov152 · 2024-11-05T14:48:59Z

Tangem/Modules/Main/MainCoordinator.swift

+        let openBuy: (URL) -> Void = { url in
+            self.openBuyCrypto(
+                at: url,
+                action: { [weak self] in
+                    self?.actionButtonsBuyCoordinator = nil
+                }
            )
+        }


И вот это тоже не очень получается, я думаю надо полностью копировать/переносить логику в ActionButtonsBuyCoordinator
У нас же сейчас дублируется логика в TokenDetailsCoordinator и в MainCoordinator и в MarketsTokenDetailsCoordinator
Видать надо дублировать и сюда
Можно пробовать их объединить в общую фабрику как нибудь, вообще подход к сборке ViewModel можно чуток поменять на фабрику, но это уже другая история

Balashov152 · 2024-11-05T14:49:34Z

Tangem/Modules/Main/MultiWalletMainContent/MultiWalletMainContentViewModel.swift

@@ -32,6 +32,10 @@ final class MultiWalletMainContentViewModel: ObservableObject {

    private(set) lazy var bottomSheetFooterViewModel = MainBottomSheetFooterViewModel()

+    var actionButtonsViewModel: ActionButtonsViewModel? {
+        makeActionButtonsViewModel()


каждый раз новая будет создаваться? И зачем опционал, как будто можно в init все сделать

m3g0byt3 · 2024-11-05T14:51:07Z

Tangem/Modules/Main/ActionButtons/Factory/ActionButtonsFactory.swift

        }
    }
 }

 private extension ActionButtonViewModel {
-    convenience init(from dataModel: ActionButtonModel, coordinator: ActionButtonsRoutable) {
+    convenience init(from dataModel: ActionButtonModel, coordinator: ActionButtonsRoutable, userWalletModel: UserWalletModel) {
        let didTapAction: () -> Void = {


Тут захватывается координатор, это ок?
Он разве не держит родительские VM, которые в свою очередь держат список action button vms

m3g0byt3 · 2024-11-05T15:25:32Z

Tangem/Modules/Main/ActionButtonsBuy/Coordinator/ActionButtonsBuyCoordinator.swift

+import Foundation
+
+final class ActionButtonsBuyCoordinator: CoordinatorObject {
+    @Injected(\.exchangeService) private var exchangeService: ExchangeService


Странно что формирование URL не в VM
URL это же в данном случае модель, а не навигация

m3g0byt3 · 2024-11-05T15:26:36Z

Tangem/Modules/Main/ActionButtonsBuy/Models/BuyTokenSelectorStrings.swift

+//  Copyright © 2024 Tangem AG. All rights reserved.
+//
+
+struct BuyTokenSelectorStrings: TokenSelectorLocalizable {


А для чего отдельная моделька, а не стринги из Localization инлайн?

m3g0byt3 · 2024-11-05T15:32:45Z

Tangem/Modules/Main/MultiWalletMainContent/MultiWalletMainContentViewModel.swift

@@ -106,6 +110,11 @@ final class MultiWalletMainContentViewModel: ObservableObject {
        }

        isUpdating = true
+
+        if FeatureProvider.isAvailable(.actionButtons) {


Лучше наверное чекать тоггл до создания actionButtonsViewModel, чтобы совсем ее не создавать, если тоггл выкл

m3g0byt3 · 2024-11-05T15:34:50Z

Tangem/UIComponents/TokenSelectorView/TokenSelectorView.swift

+import SwiftUI
+import Combine
+
+struct TokenSelectorView<


TokenSelector звучит очень дженериково
Ее реально можно использовать везде, где есть выбор токенов?

m3g0byt3 · 2024-11-05T15:36:37Z

Tangem/UIComponents/TokenSelectorView/TokenSelectorView.swift

+private extension TokenSelectorView {
+    @ViewBuilder
+    func availableSection(title: String, items: [TokenModel]) -> some View {
+        if !viewModel.searchText.isEmpty || items.isNotEmpty {


nit: Тогда уже isNotEmpty надо делать на Collection, чтобы для всех было доступно, включая String

m3g0byt3 · 2024-11-05T15:42:09Z

Tangem/UIComponents/TokenSelectorView/TokenSelectorViewModel.swift

+        let availableTokenItems = availableModels.map { tokenSelectorItemBuilder.map(from: $0, isDisabled: false) }
+        let unavailableTokenItems = unavailableModels.map { tokenSelectorItemBuilder.map(from: $0, isDisabled: true) }
+
+        Task { @MainActor [weak self] in


А main тут так как маппинг и сортировка происходит в bg?
Он действительно в bg происходит?

m3g0byt3 · 2024-11-05T15:42:22Z

Tangem/UIComponents/TokenSelectorView/TokenSelectorViewModel.swift

+
+        let unavailableTokenItems = unavailableWalletModels.map { tokenSelectorItemBuilder.map(from: $0, isDisabled: true) }
+
+        Task { @MainActor [weak self] in


Аналогичный вопрос что и выше

m3g0byt3 · 2024-11-05T15:44:07Z

Tangem/UIComponents/TokenSelectorView/TokenSelectorViewModel.swift

+        let isContainsName = item.name.lowercased().contains(text.lowercased())
+        let isContainsCurrencySymbol = item.currencySymbol.lowercased().contains(text.lowercased())


nit: есть caseInsensitiveEquals экстеншн, он по идее не будет создавать новые стринги. Можно вынести из BSDK в Foundation его и заюзать

m3g0byt3 · 2024-11-05T15:45:28Z

Tangem/UIComponents/TokenSelectorView/TokenSelectorViewModel.swift

+            .store(in: &cancellables)
+    }
+
+    func updateView(searchText: String = "") {


Мне кажется еще можно стрипать leading/trailing whitespaces, вроде как стандартная практика

Vyachesl0ve requested review from tureck1y and a team as code owners November 4, 2024 16:22

Vyachesl0ve requested review from m3g0byt3 and Balashov152 November 4, 2024 16:22

Vyachesl0ve marked this pull request as draft November 4, 2024 16:22

Vyachesl0ve force-pushed the feature/IOS-8236_token_selector branch from ebbb504 to eec6d2d Compare November 4, 2024 16:38

IOS-8236 Token selector view and buy from action buttons

3f789e9

Vyachesl0ve force-pushed the feature/IOS-8236_token_selector branch from eec6d2d to 3f789e9 Compare November 4, 2024 17:20

Vyachesl0ve commented Nov 4, 2024

View reviewed changes

Vyachesl0ve marked this pull request as ready for review November 4, 2024 17:42

Balashov152 reviewed Nov 5, 2024

View reviewed changes

Vyachesl0ve requested a review from Balashov152 November 5, 2024 14:36

IOS-8236 Review fixes

f8670ed

Vyachesl0ve force-pushed the feature/IOS-8236_token_selector branch from fe1a39f to f8670ed Compare November 5, 2024 14:44

Balashov152 reviewed Nov 5, 2024

View reviewed changes

m3g0byt3 reviewed Nov 5, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

IOS-8236 Token selector view and buy from action buttons #4161

IOS-8236 Token selector view and buy from action buttons #4161

Vyachesl0ve commented Nov 4, 2024 •

edited

Loading

Vyachesl0ve Nov 4, 2024

Balashov152 Nov 5, 2024

Vyachesl0ve Nov 5, 2024

Balashov152 Nov 5, 2024

Balashov152 Nov 5, 2024 •

edited

Loading

Balashov152 Nov 5, 2024

Balashov152 Nov 5, 2024

m3g0byt3 Nov 5, 2024

m3g0byt3 Nov 5, 2024

m3g0byt3 Nov 5, 2024

m3g0byt3 Nov 5, 2024

m3g0byt3 Nov 5, 2024

m3g0byt3 Nov 5, 2024

m3g0byt3 Nov 5, 2024

m3g0byt3 Nov 5, 2024

m3g0byt3 Nov 5, 2024

m3g0byt3 Nov 5, 2024


		final class ActionButtonsViewModel: ObservableObject {
		@Injected(\.exchangeService) private var exchangeService: ExchangeService


		let unavailableTokenItems = unavailableWalletModels.map { tokenSelectorItemBuilder.map(from: $0, isDisabled: true) }

		Task { @MainActor [weak self] in

		let isContainsName = item.name.lowercased().contains(text.lowercased())
		let isContainsCurrencySymbol = item.currencySymbol.lowercased().contains(text.lowercased())

IOS-8236 Token selector view and buy from action buttons #4161

Are you sure you want to change the base?

IOS-8236 Token selector view and buy from action buttons #4161

Conversation

Vyachesl0ve commented Nov 4, 2024 • edited Loading

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

Balashov152 Nov 5, 2024 • edited Loading

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

Vyachesl0ve commented Nov 4, 2024 •

edited

Loading

Balashov152 Nov 5, 2024 •

edited

Loading