Skip to content

Conversation

@kate-deriv
Copy link
Contributor

@kate-deriv kate-deriv commented Sep 12, 2024

Changes:

Issue N1
The issue is that styles was overwritten because of the loading order of css. When switching to Contract Details, we lazy-load component with quill styles (in Network a new chunk is added). As they arrive later, they overwrite some original css properties.
What was done: lazy loading was removed.
Issue N2
It turns out, that when Loading.DTraderV2 is rendered (from packages/core/src/App/app.jsx), html tag has no class. I checked quill-ui and quill-tokens and found out that value of the token depends on html tag class: if it's html.light - then the value of the token will be for light mode, if html.dark - for dark mode. In our case html have no class at all for the first second (so the default value of token will be dark) and only after that it gains the proper class.
What was done: added class to html tag in app.jsx (in useEffect) in order to apply it earlier

Screenshots:

Screen.Recording.2024-09-12.at.4.08.24.PM.mov

@boring-cyborg boring-cyborg bot added the Core label Sep 12, 2024
@vercel
Copy link

vercel bot commented Sep 12, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
deriv-app ✅ Ready (Inspect) Visit Preview Sep 16, 2024 10:03am

@kate-deriv kate-deriv marked this pull request as ready for review September 12, 2024 13:12
@github-actions
Copy link
Contributor

github-actions bot commented Sep 12, 2024

A production App ID was automatically generated for this PR. (log)

Click here to copy & paste above information.
- **PR**: [https://github.com/binary-com/deriv-app/pull/16829](https://github.com/binary-com/deriv-app/pull/16829)
- **URLs**:
    - **w/ App ID + Server**: https://deriv-app-git-fork-kate-deriv-kate-dtra-1880fixstyleload-a963cf.binary.sx?qa_server=red.derivws.com&app_id=24089
    - **Original**: https://deriv-app-git-fork-kate-deriv-kate-dtra-1880fixstyleload-a963cf.binary.sx
- **App ID**: `24089`

@github-actions
Copy link
Contributor

github-actions bot commented Sep 12, 2024

🚨 Lighthouse report for the changes in this PR:

Category Score
🔺 Performance 28
🟧 Accessibility 70
🟢 Best practices 92
🟧 SEO 77
🟧 PWA 78

Lighthouse ran with https://deriv-app-git-fork-kate-deriv-kate-dtra-1880fixstyleload-a963cf.binary.sx/

@coveralls
Copy link

coveralls commented Sep 12, 2024

Coverage Status

coverage: 52.232% (-0.03%) from 52.26%
when pulling 9a65512 on kate-deriv:kate/DTRA-1880/fix_style_loading_order
into d761e7f on binary-com:master.

() => <HeaderFallback />
)();

const DTraderV2ContractDetailsHeader = makeLazyLoader(
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 issue is that styles was overwritten because of the loading order of css. When switching to Contract Details, we lazy-load component with quill styles (in Network a new chunk is added). As they arrive later, they overwrite some original css properties.

vinu-deriv
vinu-deriv previously approved these changes Sep 16, 2024

const getLoader = () =>
isDTraderV2() ? (
is_dtrader_v2 ? (
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 do not want to show loader for V2 in, let's say, trader's hub as isDTraderV2() doesn't check the location

@sonarqubecloud
Copy link

);

React.useEffect(() => {
const html = document?.querySelector('html');
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 value of quill tokens is based on css selector: html.light - for light mode, html.dark - for dark mode. At the moment of loading, html tag has no class (for 1 sec), so the value of token is incorrect. Adding class in ui store allows to apply it from the very beginning.

@github-actions
Copy link
Contributor

github-actions bot commented Sep 16, 2024

Generating Lighthouse report...

@kate-deriv kate-deriv changed the title DTRA-1880 / Kate / Chunks loading order issue DTRA-1880 / Kate / [DTrader-V2] Chunks loading order issue Sep 17, 2024
@vinu-deriv vinu-deriv merged commit 64e1bef into deriv-com:master Sep 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants