[DTRA] Henry/wall 2135/wallets account switcher #606

henry-deriv · 2024-02-06T08:26:59Z

Changes:

New header for wallets

Type of change

…n click outside and close on opening of other dropdowns like platform switcher or notification

…icons and make use of them, change account list css

…u platform switcher

…count

akmal-deriv · 2024-02-16T06:55:14Z

src/javascript/_common/base/client_base.js

@@ -28,7 +28,7 @@ const ClientBase = (() => {

    const isValidLoginid = () => {
        if (!isLoggedIn()) return true;
-        const valid_login_ids = new RegExp('^(MX|MF|VRTC|MLT|CR|FOG)[0-9]+$', 'i');
+        const valid_login_ids = new RegExp('^(MX|MF|VRTC|MLT|CR|FOG|VRW|CRW)[0-9]+$', 'i');


Why do we need this change?

Wallet accounts start with VRW and CRW. Without this change, page refreshes during login which causes log out.

kate-deriv · 2024-02-16T07:16:06Z

src/javascript/_common/base/client_base.js

@@ -135,6 +135,9 @@ const ClientBase = (() => {
        return only_real_loginids.every(loginid => get('currency', loginid) && !isCryptocurrency(get('currency', loginid)));
    };

+    const isWalletsAccount = (loginid) => getAllAccountsObject()[loginid].account_category === 'wallet';


Because it is not TS, it's hard to say if loginid is defined and if it's present among keys of returned object getAllAccountsObject().

Sorry. What do u mean to say?

[loginid] accepts anything

I mean, is there opportunity to get undefined here getAllAccountsObject()[loginid]?

fair point. i handled the edge cases. please check

balakrishna-deriv · 2024-02-16T07:16:36Z

src/javascript/_common/base/livechat.js

-        window.LC_API.on_chat_ended = () => {
-            setNameEmail();
-        };
+        if (window.LC_API && window.LC_API.on_chat_ended) {


is this condition needed? - window.LC_API.on_chat_ended

Is optional chaining supported here?

yeah. Theres a console error complaining because on_chat_ended doesn't exist in LC_API object. i dont believe on_chat_ended exists in the object until live chat is started then closed.

but we are checking for this function window.LC_API.on_chat_ended before defining it at the below line

ah u r right i'll fix.

kate-deriv · 2024-02-16T07:28:22Z

src/javascript/app/base/header.js

@@ -92,6 +108,9 @@ const Header = (() => {
        applyToAllElements('.url-manage-account', el => {
            el.href = Url.urlForDeriv('redirect', `action=manage_account&ext_platform_url=${encodeURIComponent(window.location.href)}`);
        });
+        applyToAllElements('.url-wallets-deposit', el => {
+            el.href = Url.urlForDeriv('wallets/cashier/transfer', `ext_platform_url=${encodeURIComponent(window.location.href)}`);


Will it make sense to move encodeURIComponent(window.location.href) in a constant and reuse it? encodeURIComponent(window.location.href) is used in multiple lines in setHeaderUrls

ya this is fine. i'll implement.

kate-deriv · 2024-02-16T07:35:21Z

src/javascript/app/base/header.js

+            el.src = Url.urlForStatic(`${header_icon_base_path}ic-bell.svg`);
+        });
+
+        applyToAllElements('#header__notification-empty', (el) => {


Should this selectors match to that from line 111? If yes, then it should be header__notification-empty-img
Not sure, if this comment is relevant, but I noticed that all the rest selectors have a pair from constants above

yeah i was applying to container instead of image tag. i fixed it

kate-deriv · 2024-02-16T07:41:10Z

src/javascript/app/base/header.js

@@ -275,7 +318,7 @@ const Header = (() => {
        mobile_menu__livechat_logo.src = Url.urlForStatic('images/common/livechat.svg');

        // Notification Event
-        const notification_bell      = getElementById('header__notiifcation-icon-container');
+        const notification_bell      = getElementById('header__notifcation-icon-container');


notifcation ---> notification

nice catch lol.

kate-deriv · 2024-02-16T07:47:45Z

src/javascript/app/base/header.js

+        const account_switcher            = Client.hasWalletsAccount() ? getElementById('wallet__switcher') : getElementById('account__switcher');
+        const account_switcher_dropdown   = Client.hasWalletsAccount() ? getElementById('wallet__switcher-dropdown') : getElementById('account__switcher-dropdown');
+        const account_switcher_active     = Client.hasWalletsAccount() ? 'wallet__switcher-dropdown--show' : 'account__switcher-dropdown--show';
+        const wallet_switcher_close       = getElementById('wallet__switcher-close');


Does it (wallet_switcher_close) always exist or only if Client.hasWalletsAccount() === true?

Because after you put an eventListener on it

wallet_switcher_close exist only if Client.hasWalletaccount() is true. I remove the whole < WalletHeader /> from the DOM at the start in switchHeader() at the top.

Theres actually a lot of shared classnames/IDs in both <Header /> and <WalletHeader />. Removing the switcher from the DOM was the way I could attach event handler without rewriting the same thing but with different classnames. JS didn't know where to attach the event handler when classnames/IDs are same but both headers exist in the DOM.

kate-deriv · 2024-02-16T07:52:46Z

src/javascript/app/base/header.js

+                    } else {
+                        add_account_text_eu_country.style.display               = 'none';
+                    }
+                    if (isEuCountry() && has_real_account) {


I know, it's an old code and you didn't touch this part, but maybe:

if (isEuCountry()) { add_account_text_normal.style.display = 'none'; if (has_real_account) { add_account_text_eu_country.parentElement.style.display = 'none'; } } else { add_account_text_eu_country.style.display = 'none'; }

lets not touch this lol. Any fart in the wind and the whole account switcher will break. It has like 4 different account types that is juggling lol

kate-deriv · 2024-02-16T08:02:56Z

src/javascript/app/base/header.js

+                    const currency             = Client.get('currency', loginid);
+                    const currencyName         = mapCurrencyName(currency);
+
+                    const getIcon              = (() => {


Sorry, didn't get why do we need extra brackets here

thats a typo. thanks for pointing it out

src/sass/_common/wallet_header.scss

kate-deriv · 2024-02-16T08:44:26Z

src/sass/_common/wallet_header.scss

+                border-top: 1px solid var(--general-hover);
+                bottom: 0;
+                position: fixed;
+                width: stretch;


First time saw this css property for width))))

i didnt know until yesterday either

kate-deriv · 2024-02-16T08:46:02Z

src/templates/_common/_layout/header.jsx

-                </div>
-            </div>
-            <div id='mobile_menu-content' className='mobile__menu-content mobile__menu-content--active'>
-                <div className='mobile__platform-switcher' >


Is there any style which can be also deleted?

what do u mean?

I mean if you delete a component, which have classNames, we need to make sure that this classnames are reused or deleted

Okay, I can see that they are still being used in new files, sorry

yeah i just removed and turned it into a component

kate-deriv · 2024-02-16T08:55:09Z

src/templates/_common/_layout/wallet-header.jsx

+                    </a>
+                </div>
+                <div className='wallet__header-menu-item wallet__header-menu-links client_logged_in invisible mobile-hide'>
+                    <a className='url-wallet-apps'>


I don't remember exactly, but is href attribute required? I saw a lot of places, where devs were using at least dummy href : href="#"
Please, ignore if relevant

not in this case. hrefs are applied via javascript in header.js. So "#" would be wrong.

farhan-nurzi-deriv · 2024-02-16T09:38:43Z

src/javascript/_common/base/client_base.js

@@ -135,6 +135,19 @@ const ClientBase = (() => {
        return only_real_loginids.every(loginid => get('currency', loginid) && !isCryptocurrency(get('currency', loginid)));
    };

+    const isWalletsAccount = (loginid) => {
+        if (typeof loginid === 'undefined') {


undefined here is a string, is it correct?

maryia-deriv · 2024-02-16T11:47:03Z

src/sass/_common/wallet_header.scss

+            gap: 8px;
+        }
+        &-logo {
+            width: 148px !important;


which style is overriding it? is it possible to move this style after the place where it gets overriden in order to avoid using !important? or can we use some other approach to increase this CSS selector's specificity (priority) for your element without using !important? :)

i forget but let me check

this is handled! i removed this classname completely and handled the other classname with media query

…portant attribute in css

github-actions · 2024-03-11T06:54:07Z

Preview Link: https://pr-606.smarttrader-preview.pages.dev

Name	Result
Build status	Completed ✅
Preview URL	Visit Preview
Action URL	Visit Action

fix: setup new header for wallets

00e8738

henry-deriv requested a review from a team as a code owner February 6, 2024 08:27

henry-deriv added 15 commits February 8, 2024 10:03

fix: populated accounts and balance

b1e4d56

Merge branch 'master' into henry/wall-2135/wallets-account-switcher

c3447f2

fix: attach click event handlers to wallet switcher dropdown, close o…

76f6207

…n click outside and close on opening of other dropdowns like platform switcher or notification

fix: extract notification component to a separate file. Add combined …

febc40a

…icons and make use of them, change account list css

fix: switch platform switcher logos

45293b3

fix: remove need for sudo access to run the project

0fb57f5

fix: final touch up on desktop view

4eabdfc

fix: dynamically switch location of notification and style mobile men…

28351c4

…u platform switcher

fix: remove unused code

ce64f97

fix: implement wallet account swicher in dropdown

95b77e0

fix: clean up minor issues

61bce68

fix: fix mobile platform switcher when user is logged out

81280f5

fix: handle the case where account is low risk and user selects mf ac…

a7518bf

…count

fix: remove red CTA when client is logged in

a98023a

fix: set cta link to traders hub when not logged in

25dcc16

akmal-deriv reviewed Feb 16, 2024

View reviewed changes