feat(EWM-517): NFT #803
feat(EWM-517): NFT #803
Conversation
…fix fields order & validation;
…ipboardPasteButton` and use them across multiple widgets;
Odrin
force-pushed
the
feature/nft
branch
3 times, most recently
from
e661d36 to
85f926c
Compare
… with not passed parameter to compassBack
…nd handle null NFT names
| (name) => switch (name) { | ||
| 'grid' => NftDisplayMode.grid, | ||
| 'list' => NftDisplayMode.list, | ||
| _ => null, | ||
| }, |
There was a problem hiding this comment.
Мне кажется лучше сделать метод внутри enum - static NftDisplayMode byName(String name)
| sizeType: PrimaryTextFieldSizeType.medium, | ||
| hintText: LocaleKeys.nftPasteHint.tr(), | ||
| textEditingController: wm.addressController, | ||
| inputFormatters: [wm.addressFilterFormatter], |
There was a problem hiding this comment.
Зачем формировать массив в build. Можно в wm положить уже в массиве.
|
|
||
| late final _currentAccount = createNotifierFromStream(model.currentAccount); | ||
| late final _isLoading = createNotifier(false); | ||
| late final _error = createNotifier<String>(); |
There was a problem hiding this comment.
Давайте ко всем нотифаерам в название переменной добавлять State или Notifier. Так будет проще идентифицировать.
А так как в переменной лежит не bool, а notifier с bool, то вместо названия is"что-то" - loadingState/loadingNotifier.
There was a problem hiding this comment.
Hungarian notation - это устаревшая практика и антипаттерн, который снижает читаемость и дублирует информацию о типе переменной.
| crossAxisCount: 2, | ||
| mainAxisSpacing: DimensSizeV2.d8, | ||
| crossAxisSpacing: DimensSizeV2.d8, | ||
| childAspectRatio: 168 / 196, |
There was a problem hiding this comment.
Можно вынести в константу.
| ); | ||
| } | ||
|
|
||
| Widget _buildGridView( |
There was a problem hiding this comment.
Давайте максимально избегать построения изх методов.
Это антипаттерн.
There was a problem hiding this comment.
Это не построение из метода, а передача билдера в PagingListener
| ) ?? | ||
| _indicator, | ||
| if (nativeUSDPrice != null && amount != null) | ||
| AmountWidget.dollars( |
There was a problem hiding this comment.
Зачем здесь DoubleSourceBuilder?
nativeUSDPrice Нужен только тут.
Можно использовать 2 StateNotifierBuilder. Отличие в том что при изменении nativeUSDPrice не будет рендерится все поддерево, а только эта часть.
| firstSource: fees, | ||
| secondSource: feeError, | ||
| thirdSource: wm.nativeUSDPrice, | ||
| builder: (_, fees, feeError, nativeUSDPrice) { |
There was a problem hiding this comment.
Выглядит так что TripleSourceBuilder здесь не сильно нужен.
Рассмотри вариант чтобы сократить. Так как все поддерево будет рендерится реагируя на 3 notifier.
| ), | ||
| ); | ||
| } else { | ||
| Widget _buildCommentSuffix() => ValueListenableBuilder( |
There was a problem hiding this comment.
Либо встраивать в дерево, либо выносить в виджет. Но не функция.
| ); | ||
| ClipboardPasteButton( | ||
| value: addressController, | ||
| onClear: () { |
There was a problem hiding this comment.
Это Stateful - можно вынести в метод.
Odrin
force-pushed
the
feature/nft
branch
2 times, most recently
from
546e128 to
2f437f6
Compare
Description
copilot:all
Type of Change