volodymyr-lisniak / magento-2-gulp

Gulp for Magento 2. It works with core Magento styles (less) and structure. Uses default theme configs from dev/tools/grunt/configs/local-themes.js.
MIT License
41 stars 12 forks source link

Some styles miss in critical file output #24

Closed mrtuvn closed 3 years ago

mrtuvn commented 3 years ago

Is this a normal behavior ? After run gulp task gen critical i will expected file get all neccessary styles need for render page without jank or unstyles. In a reality i tested this with default luma theme of magento (not use default magento file critical) . https://github.com/magento/magento2/blob/2.4-develop/app/design/frontend/Magento/luma/web/css/critical.css

I found this file critical.css generated miss some styles relate (breadcrumbs). This link on top below nav. User while load page may notices some breaks in this component because styles not available soon enough

Here is my screenshot for reference (when page loading)

Expected results Page should load at early stage without missing styles (look like jank or broken)

Actually results Some area miss styles can see when page loading!

Screenshot from 2021-03-04 16-49-35

volodymyr-lisniak commented 3 years ago

@mrtuvn Could you show me your criticalConfig.js file?

mrtuvn commented 3 years ago

of course here you go Screenshot from 2021-03-04 20-59-15

mrtuvn commented 3 years ago

module.exports = { hostname: 'app.contribution', generic: 'test', useHttp2: true };

file local.js Not sure you can reproduce same like mine

MAYBE OUT OF SCOPE OF THIS REPORT Used ubuntu 18.10 My enviroment running in docker (nginx,php,mysql,node) node v10.22.0 npm v6.14.6

When run critical task inside dockeri got issue like this

Screenshot from 2021-03-04 21-11-06

So i have to run outside of it (right in project root folder contain my project) From host machine (outside) node v10.24.0 npm 6.14.11

Screenshot from 2021-03-04 21-16-20

volodymyr-lisniak commented 3 years ago

@mrtuvn Maybe you have broken the page at the moment when you run gulp critical --luma?

Here are my examples with different configs (criticalConfig.js)

module.exports = {
    ...
    height: 130,
    ...
};

critical.css 11KB

130
module.exports = {
    ...
    height: 900,
    ...
};

critical.css 16KB

900
mrtuvn commented 3 years ago

i have tested with luma install with sample data inside docker but somehow critical task gulp can't run inside that. So i have to run from outside host machine. Task run complete without anyproblem but the final result seem miss some styles

My expect is file output critical should be have all neccessary styles need for render page without jank. I don't have retouch/modify that file after file generated

Your results pretty much close like me

volodymyr-lisniak commented 3 years ago

@mrtuvn Does critical.css size change with different settings in your case?

module.exports = {
    ...
    height: 900,
    ...
};

critical.css 16KB

module.exports = {
    ...
    height: 130,
    ...
};

critical.css 11KB

mrtuvn commented 3 years ago

Yes i think value will be difference

volodymyr-lisniak commented 3 years ago

I think this problem is caused by the page you are tested. If you check the styles on the home page everything should be ok because critical.css created for exactly this page.

module.exports = {
    ...
    url: `${ptotocol}://${localConfig.hostname}.${localConfig.generic}/`,
    ...
};

As a solution, I can recommend creating critical styles only for the header (130px in the case of the Luma theme). And for the rest of the page, you could use the loader to hide the unstyled content before the main styles are loaded.

volodymyr-lisniak commented 3 years ago

@mrtuvn Does it satisfy your request? Can I close the issue?

mrtuvn commented 3 years ago

OK i think we can close it now. Thanks you