Skip to content

Add "Recent Wallets" to the top of the Wallet List when searching#5621

Merged
Jon-edge merged 1 commit intodevelopfrom
jon/recent-wallets-list
Jun 17, 2025
Merged

Add "Recent Wallets" to the top of the Wallet List when searching#5621
Jon-edge merged 1 commit intodevelopfrom
jon/recent-wallets-list

Conversation

@Jon-edge
Copy link
Copy Markdown
Contributor

@Jon-edge Jon-edge commented Jun 6, 2025

CHANGELOG

Does this branch warrant an entry to the CHANGELOG?

  • Yes
  • No

Dependencies

none

Requirements

If you have made any visual changes to the GUI. Make sure you have:

  • Tested on iOS device
  • Tested on Android device
  • Tested on small-screen device (iPod Touch)
  • Tested on large-screen device (tablet)

Copy link
Copy Markdown
Contributor

@swansontec swansontec left a comment

Choose a reason for hiding this comment

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

This is fine, but a bit weirdly complicated.

Comment on lines +107 to +122
return list
.map((item, index) => ({ item, index }))
.sort((a, b) => {
if (a.item.type === 'asset' && b.item.type === 'asset') {
const keyA = `${a.item.wallet.id}_${(a.item.token?.currencyCode ?? a.item.wallet.currencyInfo.currencyCode).toLowerCase()}`
const keyB = `${b.item.wallet.id}_${(b.item.token?.currencyCode ?? b.item.wallet.currencyInfo.currencyCode).toLowerCase()}`

const pa = recentIndex[keyA]
const pb = recentIndex[keyB]
if (pa != null && pb != null) return pa - pb
if (pa != null) return -1
if (pb != null) return 1
}
return a.index - b.index
})
.map(({ item }) => item)
Copy link
Copy Markdown
Contributor

@swansontec swansontec Jun 13, 2025

Choose a reason for hiding this comment

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

While this works, it's a weird way to do it. I would have done something more like this:

// Make a list of recent wallets:
let recentWallets = []
for (const {id, currencyCode} of mostRecentWallets) {
  const item = list.find(...)
  if (item != null) recentWallets.push(item)
}

// Put the recent wallets in the front:
return [
  ...recentWallets,
  ...list.filter(item => !recentWallets.includes(item))
]

However, I do see that your way works, so this is just a suggestion. I still approve your version.

@Jon-edge Jon-edge force-pushed the jon/recent-wallets-list branch 2 times, most recently from 224135c to cb37db6 Compare June 16, 2025 23:36
@Jon-edge Jon-edge force-pushed the jon/recent-wallets-list branch from cb37db6 to dbbc2aa Compare June 16, 2025 23:50
@Jon-edge Jon-edge enabled auto-merge June 16, 2025 23:58
@Jon-edge Jon-edge merged commit 7b5541e into develop Jun 17, 2025
2 checks passed
@Jon-edge Jon-edge deleted the jon/recent-wallets-list branch June 17, 2025 00:19
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.

2 participants