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.

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

Jump to bottom

Feature/multiple allow list #1210

Merged
merged 2 commits into from
Mar 26, 2024
Merged

Feature/multiple allow list #1210

merged 2 commits into from
Mar 26, 2024

Conversation

piyalbasu
Copy link
Contributor

Screen.Recording.2024-03-25.at.6.21.27.PM.mov

piyalbasu
piyalbasu commented Mar 25, 2024
View reviewed changes
.eslintrc.js
@@ -31,6 +31,7 @@ module.exports = {
"react/jsx-filename-extension": ["error", { extensions: [".tsx", ".jsx"] }],
"jsdoc/newline-after-description": "off",
"max-len": "off",
"no-await-in-loop": "off",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

we can get rid of this, right? it'll be one last lint rule to ignore in the code lol

Copy link
Contributor

Choose a reason for hiding this comment

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

😄 yes please

piyalbasu
piyalbasu commented Mar 25, 2024
View reviewed changes
@shared/api/internal.ts
@@ -1162,7 +1163,9 @@ export const editCustomNetwork = async ({
return response;
};

export const loadSettings = (): Promise<Settings & IndexerSettings> =>
export const loadSettings = (): Promise<
Settings & IndexerSettings & { assetsLists: AssetsLists }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

assets lists don't quite fall into the paradigm of general app settings or settings from the indexer. Maybe if this "3rd thing" category grows, we can add a separate service to deal with it

piyalbasu
piyalbasu commented Mar 25, 2024
View reviewed changes
@shared/api/types.ts
@@ -66,7 +68,7 @@ export interface Response {
network: string;
networkIndex: number;
networkName: string;
recentAddresses: Array<string>;
recentAddresses: string[];
Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixing a couple linting warnings while I'm in here

piyalbasu
piyalbasu commented Mar 25, 2024
View reviewed changes
extension/src/background/helpers/dataStorage.ts
@@ -254,6 +256,18 @@ export const resetAccountSubscriptions = async () => {
// once account is unlocked, setup Mercury account subscription if !HAS_ACCOUNT_SUBSCRIPTION
await localStore.setItem(HAS_ACCOUNT_SUBSCRIPTION, {});
}
await migrateDataStorageVersion("4.0.2");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the resetAccountSubscription migration forgot to migrate the version, so adding this in here now

Copy link
Contributor

Choose a reason for hiding this comment

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

omg, thank you

aristidesstaffieri
aristidesstaffieri reviewed Mar 26, 2024
View reviewed changes
@shared/constants/soroban/token.ts

export const DEFAULT_ASSETS_LISTS: AssetsLists = {
[NETWORKS.PUBLIC]: [
{
Copy link
Contributor

Choose a reason for hiding this comment

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

In the mocks, it looks like we will allow deleting a list but always want to retain one list as the "default" or one list that the user cannot delete. Should we add a field to the stellar expert list in order to key that one as the default one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, good question. I need to follow up with product on this. Not sure which of these should be "undeletable"

aristidesstaffieri
aristidesstaffieri reviewed Mar 26, 2024
View reviewed changes
extension/src/popup/views/ManageAssetsLists/index.tsx
size="md"
isFullWidth
variant="tertiary"
onClick={() => navigateTo(ROUTES.account)}
Copy link
Contributor

Choose a reason for hiding this comment

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

assuming the add list is "TODO"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup, just placeholder until the next view is created

@piyalbasu piyalbasu merged commit e8c8cf4 into release/5.18.0 Mar 26, 2024
3 checks passed
@piyalbasu piyalbasu deleted the feature/multiple-allow-list branch March 26, 2024 17:46
piyalbasu added a commit that referenced this pull request Apr 23, 2024
@piyalbasu @aristidesstaffieri
Release/5.18.0 (#1222)
a733a59 
* before we close the sign transaction window, complete metrics upload (#1209)

* Better UX for contract IDs (#1122)

* adds new component TruncateMiddle, handles dynamic truncation based on parent width

* adds font prop and tweaks operation pair to allow for width measuring

* tweaks container styles, adds use in keyval list

* adds copy to all contract IDs and keys from outside sources

* removes unused ref from previosu tries

* adds tests for truncateString helper

* fixes tests after dom changes on contract IDs

* adds copy value component, replaces inline copy instances. Uses boolean namign pattern. Only hides generic domain icon when domain is not present in asset detail

* fixes lint warnings

* uses truncated display value in KeyVal CopyValues

* Feature/multiple allow list (#1210)

* add migration for assetslist; create UI for displaying assets lists

* add translations

* adds internal getTokenDetails call, moves server and tx builder helpers to shared namespace

* replaces all inline calls to get token details with internal helper call

* replaces some instances of sorobanClient from context

* removes remaining uses of soroban client from context, remove soroban context

* adds soroban rpc not supported error

* adds sizes to error and loading asset icons to fix layout shift and align these states

* renames getIndexerTokenDetails to getTokenDetails, tweaks loading state in review auth and tweaks height setter in SlideUpModal

* Feature/check multiple tokenlist (#1221)

* first pass at adding multiple asset lists

* add verification badge

* add asset notifcation tooltip

* Added translations

* rm consoles

* update yarn

* reset debounce param

* use getTokenDetails

* fix tests

* fixes keyboard navigation for onboarding - create new wallet flow

* adds key handling for recover account form in onboarding

* adds native-like-outline mixin

* Feature/modify asset list (#1224)

* add ability to edit/delete asset lists

* Added translations

* break if unable to load schema

* add comments

* make sure to check for isEnabled when verifying (#1225)

* make sure to check for isEnabled when verifying

* don't assign the func outside of the conditional

* Feature/delete asset list modal (#1226)

* add delete asset list modal

* Added translations

* add view padding offset

* Bugfix/copy fix token list (#1227)

* copy fix for asset select token list verification

* Added translations

* fix submit error (#1228)

* default to selected network if network is verifiable (#1229)

* default to selected network if network is verifiable

* remove extraneous check

* Feature/redesign settings (#1232)

* add settings icons using svgr

* Added translations

* fix jest tests

* rollback ledger testing

* rm extra spaces

* Validate asset list on every fetch (#1234)

* Added translations

* rm console logs

* asset list UI cleanup (#1235)

* break once we found the contract in asset list (#1237)

* add a test add token asset list verification (#1236)

* address design QA (#1238)

* address design QA

* dispatch click event for createPortal button

* Bugfix/product qa 5.18.0 (#1239)

* don't reset isEnabled state; better error messaging

* add shadows to scrollable area

* Bugfix/design fixes 5.18.0 (#1240)

* add enabled/disabled sections and fix submit error

* Added translations

---------

Co-authored-by: aristides <aristides.staffieri@stellar.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aristidesstaffieri aristidesstaffieri aristidesstaffieri approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@piyalbasu @aristidesstaffieri