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

Remove non-Favorites lists #5084

Merged
merged 11 commits into from
Oct 14, 2023
Merged

Remove non-Favorites lists #5084

merged 11 commits into from
Oct 14, 2023

Conversation

benisgold
Copy link
Contributor

@benisgold benisgold commented Sep 28, 2023

Fixes APP-819

What changed (plus any additional context for devs)

In this PR:
Remove UI + logic for non-favorites lists (trending, watchlist, defi, stablecoins)

In the followup PRs:
Minor UI updates, refactor of remaining logic

Specifics:

  1. Removed (and deleted) Lists component from DiscoverHome
  2. Removed non-favorites lists from AddTokenSheet
  3. Transitioned all access/modification to favorites from userLists redux to uniswap redux. The favorites list's existence/handling in userLists redux is just a wrapper around uniswap redux.
  4. Deleted all userLists redux
  5. Deleted useUserLists hook
  6. Commented out obsolete migration involving userLists (pretty sure the empty migration itself must stay so it doesn't mess up migration tracking state)
  7. Removed obsolete i18n
  8. Removed obsolete types

Screen recordings / screenshots

Simulator.Screen.Recording.-.iPhone.14.-.2023-09-28.at.13.22.14.mp4

What to test

  • adding/removing favorites works
  • there's no trace of other lists (if there is, app will definitely break)

@linear
Copy link

linear bot commented Sep 28, 2023

APP-819 Remove UI components + tear out obsolete logic

  • remove Lists component from discover screen
  • remove ability to add to non-favorites lists on AddTokenSheet
  • remove all userLists related redux/references/types
  • simply use preexisting ( + redundant) uniswapFavorites

@benisgold benisgold changed the title @benisgold/lists3 Remove non-Favorites lists Sep 28, 2023
@benisgold benisgold marked this pull request as ready for review September 28, 2023 20:27
@benisgold
Copy link
Contributor Author

benisgold commented Sep 28, 2023

don't panic, the redux state usage that was added will be removed shortly

@jinchung
Copy link
Member

jinchung commented Oct 2, 2023

Can you also remove any relevant test for lists in e2e/discoverSheetFlow.spec.js

Copy link
Contributor

@skylarbarrera skylarbarrera left a comment

Choose a reason for hiding this comment

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

👑

@@ -305,7 +303,7 @@ const ExchangeAssetList: ForwardRefRenderFunction<
setLocalFavorite(prev => {
const address = rowData.address;
const newValue = !prev?.[address];
updateList(address, 'favorites', newValue);
dispatch(uniswapUpdateFavorites(address, newValue));
Copy link
Contributor

Choose a reason for hiding this comment

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

this is gonna have conflicts post other PR yeah?

* temp

* stuff

* rm sandbox

* finished

* rm extra line

* rm obsolete code

* rm e2e

* Remove uniswap pairs redux (#5093)

* move uniswap pairs from redux to react query

* rm uniswap pairs redux

* rm testnets util

* react query + migration

* more react query

* rm logs

* rm resources/favorites

* rm comment

* Remove favorites redux (#5104)

* tearing everything out

* fix favorites not persisting bug

* docs

* typo
Copy link
Contributor

@ibrahimtaveras00 ibrahimtaveras00 left a comment

Choose a reason for hiding this comment

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

looks good 👍🏽

@benisgold benisgold merged commit 4bcedcf into develop Oct 14, 2023
@benisgold benisgold deleted the @benisgold/lists3 branch October 14, 2023 00:54
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.

4 participants