Skip to content

fix(router): frozen layout on navigation and resize #2521

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.

Sign up for GitHub

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

Merged
CharlVS merged 6 commits into from
Feb 7, 2025
Merged

fix(router): frozen layout on navigation and resize #2521

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9f538bb
chore(deps): bump SDK version hash
takenagain Feb 5, 2025
2709ee3
fix(router): use ValueKey for MainLayout
takenagain Feb 5, 2025
8a84d8f
fix(price-charts): filter out CEX coins not in known assets list
takenagain Feb 5, 2025
327beca
chore(deps): bump SDK version hash
takenagain Feb 5, 2025
8ff69c2
refactor: remove isMobile from ValueKey and update filter comment
takenagain Feb 7, 2025
8794b15
fix(charts): Allow edge-case for multiple assets for a given ticker.
CharlVS Feb 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions lib/bloc/app_bloc_root.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,8 +193,10 @@ class AppBlocRoot extends StatelessWidget {
)..add(CoinsStarted()),
),
BlocProvider<PriceChartBloc>(
create: (context) => PriceChartBloc(binanceRepository)
..add(
create: (context) => PriceChartBloc(
binanceRepository,
komodoDefiSdk,
)..add(
const PriceChartStarted(
symbols: ['KMD'],
period: Duration(days: 30),
Expand Down
8 changes: 7 additions & 1 deletion lib/bloc/cex_market_data/price_chart/price_chart_bloc.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,19 +1,22 @@
import 'package:flutter_bloc/flutter_bloc.dart';
import 'package:komodo_cex_market_data/komodo_cex_market_data.dart';
import 'package:komodo_defi_sdk/komodo_defi_sdk.dart';
import 'package:web_dex/shared/utils/utils.dart';

import 'models/price_chart_data.dart';
import 'price_chart_event.dart';
import 'price_chart_state.dart';

class PriceChartBloc extends Bloc<PriceChartEvent, PriceChartState> {
PriceChartBloc(this.cexPriceRepository) : super(const PriceChartState()) {
PriceChartBloc(this.cexPriceRepository, this.sdk)
: super(const PriceChartState()) {
on<PriceChartStarted>(_onStarted);
on<PriceChartPeriodChanged>(_onIntervalChanged);
on<PriceChartCoinsSelected>(_onSymbolChanged);
}

final BinanceRepository cexPriceRepository;
final KomodoDefiSdk sdk;
final KomodoPriceRepository _komodoPriceRepository = KomodoPriceRepository(
cexPriceProvider: KomodoPriceProvider(),
);
Expand All @@ -30,6 +33,9 @@ class PriceChartBloc extends Bloc<PriceChartEvent, PriceChartState> {
if (state.availableCoins.isEmpty) {
final coins = (await cexPriceRepository.getCoinList())
.where((coin) => coin.currencies.contains('USDT'))
// filter out assets that are not in the known coins list to
// prevent errors when using the SDK with the `.single` property
.where((coin) => sdk.assets.assetsFromTicker(coin.id).length == 1)

@CharlVS CharlVS Feb 6, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

My intention with using the .single property instead of .first/.firstOrNull was to intentionally throw an exception if there is more than one asset for a given ticker. This was possible before the SLP project was abandoned.

Although unlikely, if the coins repo has a change pushed to it where this assumption no longer holds true, then we want to error out because there are many instances where we've assumed a given ticker will have exactly 1 or 0 coins/assets, which may wreak havoc if it's not treated as a breaking change.

Unless the surrounding context makes it redundant, please change this to:

Suggested change
// filter out assets that are not in the known coins list to
// prevent errors when using the SDK with the `.single` property
.where((coin) => sdk.assets.assetsFromTicker(coin.id).length == 1)
// filter out assets that are not in the known coins list to
// prevent errors when using the SDK with the `.single` property
.where((coin) => sdk.assets.assetsFromTicker(coin.id).length >= 1)

Also, we could add an SDK method for sdk.assets.maybeAssetsFromTicker

@takenagain takenagain Feb 7, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The await cexPriceRepository.getCoinList() function on line 34 above returns a list of coins from Binance, some of which are not in our coins repository / config. Line 38 filters out assets that are not in the “known”/available assets list, or that might have more than one asset for a given ticker.

The entire application crashes in the guest/unauthenticated Wallet page because of the .single call in getSdkAsset throwing an exception in the UI layer (no entries in the set).

I updated the comment to clarify the purpose of the filter in 8ff69c2

Comment thread
CharlVS marked this conversation as resolved.
Outdated
.map((coin) async {
double? dayChangePercent = coinPrices[coin.symbol]?.change24h;

Expand Down
4 changes: 3 additions & 1 deletion lib/router/navigators/app_router_delegate.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,9 @@ class AppRouterDelegate extends RouterDelegate<AppRoutePath>
materialPageContext = context;
return GestureDetector(
onTap: () => runDropdownDismiss(context),
child: const MainLayout(),
child: MainLayout(
key: ValueKey('${isMobile}_${routingState.selectedMenu}'),
Comment thread
CharlVS marked this conversation as resolved.
Outdated
),
);
},
),
Expand Down
6 changes: 3 additions & 3 deletions packages/komodo_ui_kit/pubspec.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ packages:
description:
path: "packages/komodo_defi_rpc_methods"
ref: dev
resolved-ref: "9c75bb7b2d3a556cec4119ad29fdb7fa93fe599e"
resolved-ref: afcdfd97b909b6f7cbd9f04c25f57a11031bcaf6
url: "https://github.com/KomodoPlatform/komodo-defi-sdk-flutter.git"
source: git
version: "0.2.0+0"
Expand All @@ -104,7 +104,7 @@ packages:
description:
path: "packages/komodo_defi_types"
ref: dev
resolved-ref: "9c75bb7b2d3a556cec4119ad29fdb7fa93fe599e"
resolved-ref: afcdfd97b909b6f7cbd9f04c25f57a11031bcaf6
url: "https://github.com/KomodoPlatform/komodo-defi-sdk-flutter.git"
source: git
version: "0.2.0+0"
Expand All @@ -113,7 +113,7 @@ packages:
description:
path: "packages/komodo_ui"
ref: dev
resolved-ref: "9c75bb7b2d3a556cec4119ad29fdb7fa93fe599e"
resolved-ref: afcdfd97b909b6f7cbd9f04c25f57a11031bcaf6
url: "https://github.com/KomodoPlatform/komodo-defi-sdk-flutter.git"
source: git
version: "0.2.0+0"
Expand Down
16 changes: 8 additions & 8 deletions pubspec.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -666,7 +666,7 @@ packages:
description:
path: "packages/komodo_coins"
ref: dev
resolved-ref: "9c75bb7b2d3a556cec4119ad29fdb7fa93fe599e"
resolved-ref: afcdfd97b909b6f7cbd9f04c25f57a11031bcaf6
url: "https://github.com/KomodoPlatform/komodo-defi-sdk-flutter.git"
source: git
version: "0.2.0+0"
Expand All @@ -675,7 +675,7 @@ packages:
description:
path: "packages/komodo_defi_framework"
ref: dev
resolved-ref: "9c75bb7b2d3a556cec4119ad29fdb7fa93fe599e"
resolved-ref: afcdfd97b909b6f7cbd9f04c25f57a11031bcaf6
url: "https://github.com/KomodoPlatform/komodo-defi-sdk-flutter.git"
source: git
version: "0.2.0"
Expand All @@ -684,7 +684,7 @@ packages:
description:
path: "packages/komodo_defi_local_auth"
ref: dev
resolved-ref: "9c75bb7b2d3a556cec4119ad29fdb7fa93fe599e"
resolved-ref: afcdfd97b909b6f7cbd9f04c25f57a11031bcaf6
url: "https://github.com/KomodoPlatform/komodo-defi-sdk-flutter.git"
source: git
version: "0.2.0+0"
Expand All @@ -693,7 +693,7 @@ packages:
description:
path: "packages/komodo_defi_rpc_methods"
ref: dev
resolved-ref: "9c75bb7b2d3a556cec4119ad29fdb7fa93fe599e"
resolved-ref: afcdfd97b909b6f7cbd9f04c25f57a11031bcaf6
url: "https://github.com/KomodoPlatform/komodo-defi-sdk-flutter.git"
source: git
version: "0.2.0+0"
Expand All @@ -702,7 +702,7 @@ packages:
description:
path: "packages/komodo_defi_sdk"
ref: dev
resolved-ref: "9c75bb7b2d3a556cec4119ad29fdb7fa93fe599e"
resolved-ref: afcdfd97b909b6f7cbd9f04c25f57a11031bcaf6
url: "https://github.com/KomodoPlatform/komodo-defi-sdk-flutter.git"
source: git
version: "0.2.0+0"
Expand All @@ -711,7 +711,7 @@ packages:
description:
path: "packages/komodo_defi_types"
ref: dev
resolved-ref: "9c75bb7b2d3a556cec4119ad29fdb7fa93fe599e"
resolved-ref: afcdfd97b909b6f7cbd9f04c25f57a11031bcaf6
url: "https://github.com/KomodoPlatform/komodo-defi-sdk-flutter.git"
source: git
version: "0.2.0+0"
Expand All @@ -727,7 +727,7 @@ packages:
description:
path: "packages/komodo_ui"
ref: dev
resolved-ref: "9c75bb7b2d3a556cec4119ad29fdb7fa93fe599e"
resolved-ref: afcdfd97b909b6f7cbd9f04c25f57a11031bcaf6
url: "https://github.com/KomodoPlatform/komodo-defi-sdk-flutter.git"
source: git
version: "0.2.0+0"
Expand All @@ -743,7 +743,7 @@ packages:
description:
path: "packages/komodo_wallet_build_transformer"
ref: dev
resolved-ref: "9c75bb7b2d3a556cec4119ad29fdb7fa93fe599e"
resolved-ref: afcdfd97b909b6f7cbd9f04c25f57a11031bcaf6
url: "https://github.com/KomodoPlatform/komodo-defi-sdk-flutter.git"
source: git
version: "0.2.0+0"
Expand Down