Skip to content
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.

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

nfts sort direction #6068

Merged

Conversation

greg-schrammel
Copy link
Contributor

@greg-schrammel greg-schrammel commented Aug 30, 2024

Fixes APP-####

What changed (plus any additional context for devs)

adds options to order asc/desc to collectibles

Screen recordings / screenshots

Screen.Recording.2024-09-10.at.12.55.55.mov

What to test

Copy link

linear bot commented Aug 30, 2024

@greg-schrammel greg-schrammel marked this pull request as draft August 30, 2024 20:19
@greg-schrammel greg-schrammel marked this pull request as ready for review September 4, 2024 03:49
Copy link

socket-security bot commented Sep 4, 2024

New and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Package New capabilities Transitives Size Publisher
npm/[email protected] None 0 21.9 MB typescript-bot

🚮 Removed packages: npm/[email protected]

View full report↗︎

@jinchung jinchung removed their request for review September 5, 2024 18:36
@brunobar79
Copy link
Member

Launch in simulator or device for 433c00c

{
actionKey: 'cancel',
actionTitle: i18n.t(i18n.l.transaction_details.actions_menu.cancel),
menuAttributes: ['destructive' as const],
Copy link
Contributor

Choose a reason for hiding this comment

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

is this necessary to cast as const

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, it considers 'destructive' as string but the type wants the it as the string literal type destructive, same stuff for the others as const or satisfies added, I expanded the type a lil bit, but in these cases opted to keep it like this so we have better auto complete

Comment on lines 183 to 189
<ContextMenuButton
activeOpacity={0}
enableContextMenu
isAnchoredToRight
isMenuPrimaryAction
menuConfig={menuConfig}
onPressMenuItem={handlePressMenuItem}
useActionSheetFallback={false}
wrapNativeComponent={false}
>
Copy link
Contributor

Choose a reason for hiding this comment

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

why were these props removed?

Copy link
Contributor Author

@greg-schrammel greg-schrammel Sep 17, 2024

Choose a reason for hiding this comment

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

added isAnchoredToRight back didn't notice it's used in the .android, the others are already the default inside <ContextMenuButton />

Copy link
Contributor Author

Choose a reason for hiding this comment

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

btw seems like this GasButton menu is not working at all in android? idk if it's just my simulator

@walmat
Copy link
Contributor

walmat commented Sep 12, 2024

Screenshot 2024-09-12 at 1 30 02 PM

On Android the context menu is going offscreen a bit. I'd say this is low priority since I'm reworking context menus anyway, so dw about it, but just wanted to flag this.

@brunobar79
Copy link
Member

Launch in simulator or device for f3f5f10

Copy link
Member

@benisgold benisgold left a comment

Choose a reason for hiding this comment

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

idk if this is a bug or not, but i'd expect the "grayed out" option to be the one that's already selected which is currently not the case

Simulator.Screen.Recording.-.iPhone.15.Pro.-.2024-09-17.at.11.34.10.mp4

brunobar79 and others added 24 commits October 16, 2024 01:37
* fix wc v2 change account issues

* Update src/walletConnect/index.tsx

* Update src/components/walletconnect-list/WalletConnectV2ListItem.tsx

* Update src/components/walletconnect-list/WalletConnectV2ListItem.tsx

* fix deps array
* init apechain support

* fix wrong images

* Apply suggestions from code review

* Update chain badge

* Update chain colors

* Add all badge assets

* add backend networks runtime

* added back internal flag

* remove unused NATIVE_ASSETS_PER_CHAIN

* fix some discrepancies

---------

Co-authored-by: Christian Baroni <[email protected]>
* analytics changes

* idk

* watched wallets cohort + properly track watched wallets
* fix crash

* destructure in same line
* prevent mwp flow from failing if failed to fetch dapp metadata

* prevent logging user rejections

* ignore logging some user errors
* fix networks crash

* remove duplicated walletconnect chain ids

* use all supported chains for walletconnect

* fix weird space on approval sheet

* Update src/screens/WalletConnectApprovalSheet.tsx
* hold -> tap

* debounce
* chore: update swaps

* chore: replace extraneous crosschain logic with needsAllowance field

* chore: get rid of all WRAPPED_ASSET references and use quote swap type

* chore: remove swapType as a param to get quotes

* chore: replace swap unlock logic with needsAllowance field

* chore: code review changes
* zeego setup

* bump react-native-menu/menu

* fix android settings menu

* update deps

* fix type inference

* mvp to fix android context menus

* fix some inconsistencies on Android

* remove debug logs

* Update src/components/asset-list/RecyclerAssetList2/core/RawRecyclerList.tsx

* Update src/components/DropdownMenu.tsx

* remove unused deps

* fix build maybe

* update lock
* fixes

* fix experimental config usage

* nits

* use setTimeout

* defaultConfig fix
@brunobar79
Copy link
Member

Launch in simulator or device for f2bede0

@brunobar79
Copy link
Member

Launch in simulator or device for 42c906d

@greg-schrammel greg-schrammel merged commit 3897001 into develop Oct 22, 2024
8 checks passed
@greg-schrammel greg-schrammel deleted the gregs/app-1756-sorting-nfts-on-android-is-broken branch October 22, 2024 17:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants