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

UX: Added Account list pinning and Unpinning #21865

Merged
merged 31 commits into from
Dec 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
708be97
added controller change
NidhiKJha Nov 16, 2023
f8548de
added logic for pinning of accounts
NidhiKJha Nov 16, 2023
5205b71
added logic for pin and unpin
NidhiKJha Nov 17, 2023
92fe2c8
nit fixes
NidhiKJha Nov 17, 2023
638574a
add close menu
NidhiKJha Nov 17, 2023
1d098e8
added feature flag
NidhiKJha Nov 17, 2023
8d86198
updated selector
NidhiKJha Nov 24, 2023
cd21f13
snapshot updated for spec files
NidhiKJha Nov 24, 2023
9b2951e
fixture update
NidhiKJha Nov 28, 2023
8b98cd3
nit fix
NidhiKJha Nov 30, 2023
63a8f9a
updated ternary
NidhiKJha Nov 30, 2023
7770e8a
lint fix
NidhiKJha Dec 4, 2023
c9acad3
updated spec
NidhiKJha Dec 4, 2023
874ebb0
updated json
NidhiKJha Dec 4, 2023
5f375bc
updated json
NidhiKJha Dec 4, 2023
652c931
updated json
NidhiKJha Dec 4, 2023
f9a5437
updated logic for pinning
NidhiKJha Dec 5, 2023
e717242
fixed fencing
NidhiKJha Dec 5, 2023
16ba824
added selector test
NidhiKJha Dec 5, 2023
cc0b197
test update
NidhiKJha Dec 5, 2023
a757a08
updated jest
NidhiKJha Dec 6, 2023
94cc61f
separate pinning and unpinning
NidhiKJha Dec 11, 2023
b391525
added selector test
NidhiKJha Dec 11, 2023
267cca2
Merge branch 'develop' into fix-account-pinning-20918
NidhiKJha Dec 11, 2023
6991c52
Merge branch 'develop' into fix-account-pinning-20918
NidhiKJha Dec 11, 2023
f965d24
Merge branch 'develop' into fix-account-pinning-20918
NidhiKJha Dec 12, 2023
1012040
Merge branch 'develop' into fix-account-pinning-20918
NidhiKJha Dec 12, 2023
900d125
Merge branch 'develop' into fix-account-pinning-20918
NidhiKJha Dec 12, 2023
7c42b09
Merge branch 'develop' into fix-account-pinning-20918
NidhiKJha Dec 12, 2023
3447a2b
Merge branch 'develop' into fix-account-pinning-20918
NidhiKJha Dec 12, 2023
b9cfbc9
Merge branch 'develop' into fix-account-pinning-20918
NidhiKJha Dec 13, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .storybook/test-data.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ const state = {
},
},
orderedNetworkList: [],
pinnedAccountList: [],
tokenList: {
'0x514910771af9ca656af840dff83e8264ecf986ca': {
address: '0x514910771af9ca656af840dff83e8264ecf986ca',
Expand Down
6 changes: 6 additions & 0 deletions app/_locales/en/messages.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions app/images/icons/pin.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
4 changes: 4 additions & 0 deletions app/images/icons/unpin.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
96 changes: 96 additions & 0 deletions app/scripts/controllers/account-order.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
import {
BaseControllerV2,
RestrictedControllerMessenger,
} from '@metamask/base-controller';

// Unique name for the controller
const controllerName = 'AccountOrderController';

/**
* The account ID of a account.
*/
export type AccountAddress = string;

// State shape for AccountOrderController
export type AccountOrderControllerState = {
pinnedAccountList: AccountAddress[];
NidhiKJha marked this conversation as resolved.
Show resolved Hide resolved
};

// Describes the action for updating the accounts list
export type AccountOrderControllerupdateAccountsListAction = {
type: `${typeof controllerName}:updateAccountsList`;
handler: AccountOrderController['updateAccountsList'];
};

// Union of all possible actions for the messenger
export type AccountOrderControllerMessengerActions =
AccountOrderControllerupdateAccountsListAction;

// Type for the messenger of AccountOrderController
export type AccountOrderControllerMessenger = RestrictedControllerMessenger<
typeof controllerName,
AccountOrderControllerMessengerActions,
never,
never,
never
>;

// Default state for the controller
const defaultState = {
pinnedAccountList: [],
};

// Metadata for the controller state
const metadata = {
pinnedAccountList: {
persist: true,
anonymous: true,
},
};

/**
* Controller that updates the order of the account list.
* This controller subscribes to account state changes and ensures
* that the account list is updated based on the latest account configurations.
*/
export class AccountOrderController extends BaseControllerV2<
typeof controllerName,
AccountOrderControllerState,
AccountOrderControllerMessenger
> {
/**
* Creates a AccountOrderController instance.
*
* @param args - The arguments to this function.
* @param args.messenger - Messenger used to communicate with BaseV2 controller.
* @param args.state - Initial state to set on this controller.
*/
constructor({
messenger,
state,
}: {
messenger: AccountOrderControllerMessenger;
state?: AccountOrderControllerState;
}) {
// Call the constructor of BaseControllerV2
super({
messenger,
metadata,
name: controllerName,
state: { ...defaultState, ...state },
});
}

/**
* Updates the accounts list in the state with the provided list of accounts.
*
* @param accountList - The list of accounts to update in the state.
*/

updateAccountsList(accountList: []) {
this.update((state) => {
state.pinnedAccountList = accountList;
return state;
});
}
}
3 changes: 3 additions & 0 deletions app/scripts/lib/setupSentry.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,9 @@ export const SENTRY_BACKGROUND_STATE = {
NetworkOrderController: {
orderedNetworkList: [],
},
AccountOrderController: {
pinnedAccountList: [],
},
AppMetadataController: {
currentAppVersion: true,
currentMigrationVersion: true,
Expand Down
22 changes: 22 additions & 0 deletions app/scripts/metamask-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,7 @@ import {
import createOriginMiddleware from './lib/createOriginMiddleware';
import createTabIdMiddleware from './lib/createTabIdMiddleware';
import { NetworkOrderController } from './controllers/network-order';
import { AccountOrderController } from './controllers/account-order';
import createOnboardingMiddleware from './lib/createOnboardingMiddleware';
import { setupMultiplex } from './lib/stream-utils';
import EnsController from './controllers/ens';
Expand Down Expand Up @@ -842,6 +843,15 @@ export default class MetamaskController extends EventEmitter {
messenger: networkOrderMessenger,
state: initState.NetworkOrderController,
});

const accountOrderMessenger = this.controllerMessenger.getRestricted({
name: 'AccountOrderController',
});
this.accountOrderController = new AccountOrderController({
messenger: accountOrderMessenger,
state: initState.AccountOrderController,
});

// token exchange rate tracker
this.tokenRatesController = new TokenRatesController(
{
Expand Down Expand Up @@ -1936,6 +1946,7 @@ export default class MetamaskController extends EventEmitter {
SubjectMetadataController: this.subjectMetadataController,
AnnouncementController: this.announcementController,
NetworkOrderController: this.networkOrderController,
AccountOrderController: this.accountOrderController,
GasFeeController: this.gasFeeController,
TokenListController: this.tokenListController,
TokensController: this.tokensController,
Expand Down Expand Up @@ -1988,6 +1999,7 @@ export default class MetamaskController extends EventEmitter {
SubjectMetadataController: this.subjectMetadataController,
AnnouncementController: this.announcementController,
NetworkOrderController: this.networkOrderController,
AccountOrderController: this.accountOrderController,
GasFeeController: this.gasFeeController,
TokenListController: this.tokenListController,
TokensController: this.tokensController,
Expand Down Expand Up @@ -3174,6 +3186,7 @@ export default class MetamaskController extends EventEmitter {
markNotificationsAsRead: this.markNotificationsAsRead.bind(this),
updateCaveat: this.updateCaveat.bind(this),
updateNetworksList: this.updateNetworksList.bind(this),
updateAccountsList: this.updateAccountsList.bind(this),
getPhishingResult: async (website) => {
await phishingController.maybeUpdateState();

Expand Down Expand Up @@ -5504,6 +5517,15 @@ export default class MetamaskController extends EventEmitter {
}
};

updateAccountsList = (pinnedAccountList) => {
try {
this.accountOrderController.updateAccountsList(pinnedAccountList);
} catch (err) {
log.error(err.message);
throw err;
}
};

rejectPermissionsRequest = (requestId) => {
try {
this.permissionController.rejectPermissionsRequest(requestId);
Expand Down
1 change: 0 additions & 1 deletion builds.yml
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,6 @@ env:
# Determines if feature flagged Multichain UI should be used
- MULTICHAIN: ''
- NETWORK_ACCOUNT_DND: ''

###
# Meta variables
###
Expand Down
1 change: 1 addition & 0 deletions test/data/mock-state.json
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,7 @@
"useNftDetection": true,
"openSeaEnabled": true,
"orderedNetworkList": [],
"pinnedAccountList": [],
"advancedGasFee": {
"0x5": {
"maxBaseFee": "75",
Expand Down
8 changes: 8 additions & 0 deletions test/e2e/fixture-builder.js
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,9 @@ function defaultFixture() {
NetworkOrderController: {
orderedNetworkList: [],
},
AccountOrderController: {
pinnedAccountList: [],
},
AppStateController: {
browserEnvironment: {},
nftsDropdownState: {},
Expand Down Expand Up @@ -487,6 +490,11 @@ class FixtureBuilder {
return this;
}

withAccountOrderController(data) {
merge(this.fixture.data.AccountOrderController, data);
return this;
}

withAppStateController(data) {
merge(this.fixture.data.AppStateController, data);
return this;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
{
"AccountOrderController": { "pinnedAccountList": {} },
"AccountTracker": {
"accounts": "object",
"accountsByChainId": "object"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@
"subjectMetadata": "object",
"announcements": "object",
"orderedNetworkList": { "0": "string", "1": "string", "2": "string" },
"pinnedAccountList": {},
"gasFeeEstimates": {},
"gasFeeEstimatesByChainId": {},
"estimatedGasFeeTimeBounds": {},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
},
"AnnouncementController": { "announcements": "object" },
"NetworkOrderController": { "orderedNetworkList": {} },
"AccountOrderController": { "pinnedAccountList": {} },
"AppStateController": {
"browserEnvironment": {},
"nftsDropdownState": {},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
},
"AnnouncementController": { "announcements": "object" },
"NetworkOrderController": { "orderedNetworkList": {} },
"AccountOrderController": { "pinnedAccountList": {} },
"AppStateController": {
"browserEnvironment": {},
"nftsDropdownState": {},
Expand Down
2 changes: 2 additions & 0 deletions ui/components/component-library/icon/icon.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ export enum IconName {
Notification = 'notification',
PasswordCheck = 'password-check',
People = 'people',
Pin = 'pin',
ProgrammingArrows = 'programming-arrows',
Custody = 'custody',
Question = 'question',
Expand Down Expand Up @@ -162,6 +163,7 @@ export enum IconName {
Twitter = 'twitter',
QrCode = 'qr-code',
UserCheck = 'user-check',
Unpin = 'unpin',
Ban = 'ban',
Bold = 'bold',
CircleX = 'circle-x',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
getCurrentChainId,
getHardwareWalletType,
getAccountTypeForKeyring,
getPinnedAccountsList,
///: BEGIN:ONLY_INCLUDE_IF(build-mmi)
getMetaMaskAccountsOrdered,
///: END:ONLY_INCLUDE_IF
Expand All @@ -31,7 +32,7 @@ import {
MetaMetricsEventCategory,
MetaMetricsEventName,
} from '../../../../shared/constants/metametrics';
import { showModal } from '../../../store/actions';
import { showModal, updateAccountsList } from '../../../store/actions';
import { TextVariant } from '../../../helpers/constants/design-system';
import { formatAccountType } from '../../../helpers/utils/metrics';
import { AccountDetailsMenuItem, ViewExplorerMenuItem } from '..';
Expand All @@ -45,6 +46,7 @@ export const AccountListItemMenu = ({
isRemovable,
identity,
isOpen,
isPinned,
}) => {
const t = useI18nContext();
const trackEvent = useContext(MetaMetricsContext);
Expand All @@ -59,6 +61,8 @@ export const AccountListItemMenu = ({
);
const accountType = formatAccountType(getAccountTypeForKeyring(keyring));

const pinnedAccountList = useSelector(getPinnedAccountsList);

///: BEGIN:ONLY_INCLUDE_IF(build-mmi)
const isCustodial = keyring?.type ? /Custody/u.test(keyring.type) : false;
const accounts = useSelector(getMetaMaskAccountsOrdered);
Expand Down Expand Up @@ -121,6 +125,18 @@ export const AccountListItemMenu = ({
};
}, [handleClickOutside]);

const handlePinning = (address) => {
const updatedPinnedAccountList = [...pinnedAccountList, address];
dispatch(updateAccountsList(updatedPinnedAccountList));
};

const handleUnpinning = (address) => {
const updatedPinnedAccountList = pinnedAccountList.filter(
(item) => item !== address,
);
dispatch(updateAccountsList(updatedPinnedAccountList));
};
NidhiKJha marked this conversation as resolved.
Show resolved Hide resolved

return (
<Popover
className="multichain-account-list-item-menu__popover"
Expand All @@ -147,6 +163,22 @@ export const AccountListItemMenu = ({
textProps={{ variant: TextVariant.bodySm }}
address={identity.address}
/>
{process.env.NETWORK_ACCOUNT_DND ? (
<MenuItem
data-testid="account-list-menu-pin"
onClick={() => {
isPinned
? handleUnpinning(identity.address)
: handlePinning(identity.address);
onClose();
}}
iconName={isPinned ? IconName.Unpin : IconName.Pin}
>
<Text variant={TextVariant.bodySm}>
{isPinned ? t('unpin') : t('pinToTop')}
</Text>
</MenuItem>
) : null}
{isRemovable ? (
<MenuItem
ref={removeAccountItemRef}
Expand Down Expand Up @@ -241,6 +273,10 @@ AccountListItemMenu.propTypes = {
* Represents if the account should be removable
*/
isRemovable: PropTypes.bool.isRequired,
/**
* Represents pinned accounts
*/
isPinned: PropTypes.bool,
/**
* Identity of the account
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ exports[`AccountListItem renders AccountListItem component and shows account nam
class="mm-box mm-box--display-flex mm-box--justify-content-space-between"
>
<div
class="mm-box multichain-account-list-item__account-name mm-box--margin-inline-end-2"
class="mm-box multichain-account-list-item__account-name mm-box--margin-inline-end-2 mm-box--display-flex mm-box--gap-2 mm-box--align-items-center"
>
<button
class="mm-box mm-text multichain-account-list-item__account-name__button mm-text--body-md-medium mm-text--ellipsis mm-text--text-align-left mm-box--padding-0 mm-box--width-full mm-box--color-text-default mm-box--background-color-transparent"
Expand Down
Loading
Loading