Farabi/grwt-9029/v2-migration-banner

[farabi-deriv]

This pull request introduces a new modal dialog to notify users when the platform has been migrated and prompts them to log in again. The modal is triggered based on a specific server response and includes UI, logic, and styling updates.

Migration Modal Feature

• Added a new MigrationModal React component in migration-modal.jsx that displays a modal with login and support options when the server indicates the platform has been migrated.
• Integrated the MigrationModal into the socket response handler in socket_general.js to show the modal when the website_status.message is 'migrated'.

Styling Updates

• Created a new SCSS file migration-modal.scss for styling the migration modal, including layout, typography, and button styles.
• Imported the new migration modal styles into the main stylesheet common.scss.

[github-actions]

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

OpenSSF Scorecard

Package	Version	Score	Details

Scanned Manifest Files

[github-actions]

Claude finished @farabi-deriv's task in 2m 15s —— View job

---

PR Review: V2 Migration Banner

• Gather context and understand the request
• Read changed files
• Review migration-modal.jsx
• Review socket_general.js
• Review migration-modal.scss
• Review common.scss
• Check for security, correctness, and quality issues
• Post comprehensive review

---

Summary

The PR adds a one-time migration modal triggered by a website_status.message === 'migrated' WebSocket response. The overall approach is reasonable, but there are several issues ranging from a mandatory repo convention violation to a correctness bug that could silently break other features.

---

🔴 High Priority

1. Missing [AI] markers — mandatory repo convention

Per CLAUDE.md, all AI-generated code must be wrapped in [AI] markers. None of the three new files contain them. This is a hard repo requirement.

All new blocks in migration-modal.jsx, migration-modal.scss, and the added lines in socket_general.js must be wrapped. Example:

// [AI]
const getHomeLoginUrl = () => { ... };
// [AI]

Fix this →

---

2. Early return swallows the entire website_status response — correctness bug

socket_general.js:42-45

if (response.website_status.message === 'migrated') {
    MigrationModal.show();
    return;   // ← skips everything below
}

If the API returns { message: 'migrated', site_status: 'up', clients_country: 'gb', ... } in the same message, the early return means:

• setCurrencies() is never called
• Language dropdown is never built
• BinarySocket.setAvailability() is never called
• landing_company is never sent for logged-out users

If the migration status always arrives before these fields are populated via other responses, this may be benign — but it is fragile. Consider not returning early (let the rest of the handler run) and simply showing the modal:

if (response.website_status.message === 'migrated') {
    MigrationModal.show();
    // do NOT return — let other processing continue
}

Fix this →

---

3. MigrationModal.remove() is exported but never called — PJAX leak

migration-modal.jsx:94-100

The remove() method exists but nothing calls it. In SmartTrader's PJAX architecture (binary_pjax.js), the body is reused across page navigations. The container div#migration_modal_container appended to document.body will persist indefinitely once shown, even across PJAX navigations.

You should either hook remove() into onUnload for the relevant page modules, or accept that the modal is permanent (which seems to be the intent for a forced migration). If intentional, the remove() method is dead code and should be removed to avoid confusion.

Fix this →

---

🟡 Medium Priority

4. is_mobile read once at mount — won't respond to resize

migration-modal.jsx:24

const is_mobile = window.innerWidth < 770;

This is evaluated once when the component first renders. A user on a narrow browser who widens it (or vice versa) will see the wrong layout. Since this is a blocking modal and the user can't dismiss it, this may be acceptable in practice — but it's worth noting. Using a useEffect + ResizeObserver, or just CSS media queries in the SCSS, would be more robust.

---

5. Mixed ES module / CommonJS style

migration-modal.jsx:1-5

import React from 'react';
import ReactDOM from 'react-dom';
import { Button, Modal } from '@deriv-com/quill-ui';

const { localize } = require('../../../javascript/_common/localize');

The project convention (per CLAUDE.md) is to use require for all imports. React and ReactDOM should use require too:

const React                  = require('react');
const ReactDOM               = require('react-dom');
const { Button, Modal }      = require('@deriv-com/quill-ui');
const { localize }           = require('../../../javascript/_common/localize');

Fix this →

---

6. WebSocket subscription keeps firing after migration

After MigrationModal.show() is called, the subscribe: 1 on website_status means subsequent server pushes will keep hitting the onMessage handler. Repeated calls to show() are guarded (no-op if container exists), but the socket stays active. If you want a clean state, consider unsubscribing or closing the socket after showing the modal.

---

7. No test coverage

The new socket_general.js branch (message === 'migrated') has no mocha tests. A test asserting that MigrationModal.show() is called when the message matches, and that it is not called for normal responses, would protect against regressions.

---

🔵 Low Priority

8. !important in SCSS to fight quill-ui specificity

migration-modal.scss:42

color: $COLOR_DRIV_RED !important;

The stylelint-disable comment acknowledges this is a workaround. If quill-ui requires an !important override, it's worth filing a scoped CSS-variable override or a targeted selector with higher specificity instead, to avoid !important leakage.

---

9. Hardcoded help-centre URL path

migration-modal.jsx:20

return `https://trade.deriv.${tld}/help-centre`;

The URL is hardcoded to trade.deriv.${tld} (a different subdomain from the login URL). Verify this is intentional and matches the actual help-centre location for each TLD.

---

10. CSS variable fallback may not apply on all themes

migration-modal.scss:3-6

background-color: var(--semantic-color-slate-solid-surface-normal-low, #f2f3f4);

The CSS custom property comes from quill-ui's token system. If the token isn't injected (e.g., in non-quill contexts or tests), the fallback #f2f3f4 takes over. This is fine, just make sure themes that need a dark-mode variant are handled if applicable.

---

✅ What looks good

• Guard in MigrationModal.show() prevents double-rendering (if (container) return)
• rel='noopener noreferrer' correctly applied to the target='_blank' support link
• SCSS variables ($FONT_SIZE_XL, $FONT_SIZE_N, $FONT_SIZE_S, $COLOR_DRIV_RED) are all defined in constants.scss
• quill-ui is a declared project dependency
• disableCloseOnOverlay and showCrossIcon={false} correctly make the modal non-dismissible

[github-actions]

Claude finished @farabi-deriv's task in 2m 3s —— View job

---

PR Review: V2 Migration Banner

• Gather context and understand the request
• Read changed files
• Review migration-modal.jsx
• Review socket_general.js
• Review migration-modal.scss
• Review common.scss
• Check for security, correctness, and quality issues
• Post comprehensive review

---

Summary

The PR adds a non-dismissible migration modal triggered when the server returns website_status.message === 'migrated'. The approach is sound, but several issues from the previous review remain unaddressed, and there is one new concern. Listed by priority below.

---

🔴 High Priority

1. Missing [AI] markers — mandatory repo convention

Per CLAUDE.md, all AI-generated code must be wrapped in [AI] markers. None of the three new/modified files contain them. Every new function, block, and JSX structure in migration-modal.jsx, migration-modal.scss, and the added lines in socket_general.js must be wrapped.

Example:

// [AI]
const getHomeLoginUrl = () => { ... };
// [AI]

Fix this →

---

2. Early return skips the entire website_status payload — correctness bug

socket_general.js:42–45

if (response.website_status.message === 'migrated') {
    MigrationModal.show();
    return;   // ← setCurrencies, language dropdown, setAvailability, landing_company all skipped
}

If the API includes site_status, clients_country, or currency data in the same response message, they are silently dropped. Remove the early return so the rest of the handler still runs:

if (response.website_status.message === 'migrated') {
    MigrationModal.show();
    // no return — let remaining processing continue
}

Fix this →

---

3. Mixed module system — import / export default vs project convention

migration-modal.jsx:1–5, 105

import React from 'react';           // ← ES module
import ReactDOM from 'react-dom';     // ← ES module
import { Button, Modal } from '@deriv-com/quill-ui'; // ← ES module

const { localize } = require('...');  // ← CommonJS
...
export default MigrationModal;        // ← ES module export

The project uses CommonJS (require / module.exports) throughout. socket_general.js compensates with .default on the require, but this is fragile and inconsistent. Replace with:

const React                  = require('react');
const ReactDOM               = require('react-dom');
const { Button, Modal }      = require('@deriv-com/quill-ui');
const { localize }           = require('../../../javascript/_common/localize');
...
module.exports = MigrationModal;
// and in socket_general.js: require('...migration-modal.jsx')  (no .default)

Fix this →

---

🟡 Medium Priority

4. remove() is exported but never called — dead code / PJAX leak

migration-modal.jsx:94–100

The remove() method is never called anywhere. In SmartTrader's PJAX architecture, the div#migration_modal_container appended to document.body will persist across page navigations indefinitely. Either wire remove() into the onUnload lifecycle of relevant page modules, or delete remove() entirely if the modal is intentionally permanent (which seems to be the design intent here).

Fix this →

---

5. is_mobile evaluated once at mount — won't adapt to resize

migration-modal.jsx:24

const is_mobile = window.innerWidth < 770;

This is computed once when ReactDOM.render() is called. A user who resizes their browser won't get the correct mobile/desktop modal variant. Since this is a blocking modal the user cannot dismiss, it may be acceptable in practice — but using a CSS media query in the SCSS for responsive behaviour would be more robust.

---

6. CSS variable missing fallback — new regression

migration-modal.scss:3

background-color: var(--semantic-color-slate-solid-surface-normal-low);

There is no fallback value. If the quill-ui CSS token isn't injected (non-quill contexts, unit tests, future theme changes), the header element will have no background color. Add a fallback:

background-color: var(--semantic-color-slate-solid-surface-normal-low, #f2f3f4);

Fix this →

---

7. No test coverage for the new migration branch

The message === 'migrated' branch in socket_general.js has no mocha tests. A test asserting that MigrationModal.show() is called when response.website_status.message === 'migrated', and that it is not called for normal website_status responses, would protect against future regressions.

---

🔵 Low Priority

8. getHelpCentreUrl() points to trade.deriv.${tld} — verify intent

migration-modal.jsx:20

return `https://trade.deriv.${tld}/help-centre`;

The login URL uses home.deriv.${tld} while the support link uses trade.deriv.${tld}. Confirm this is intentional — that the help centre is specifically hosted on DTrader's subdomain and not, for example, deriv.${tld}/help-centre.

---

9. !important in SCSS to override quill-ui specificity

migration-modal.scss:33

The stylelint-disable comment acknowledges this is a workaround. A more targeted selector with higher specificity (e.g., .migration-modal .migration-modal__support-link) would avoid needing !important.

---

✅ What looks good

• Double-render guard in show() (if (container) return) prevents stacking modals
• rel='noopener noreferrer' correctly applied to the target='_blank' support link
• disableCloseOnOverlay + showCrossIcon={false} correctly make the modal non-dismissible
• All referenced SCSS variables ($FONT_SIZE_XL, $FONT_SIZE_N, $FONT_SIZE_S, $COLOR_DRIV_RED) confirmed present in constants.scss
• SmartTrader logo image included in the PR
• quill-ui is a declared project dependency

---

[github-actions]

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

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