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.

Sign up for GitHub

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

Jump to bottom

[Issue] Initialize inline translations module only when they are enabled #29553

Closed
4 tasks
m2-assistant bot opened this issue Aug 14, 2020 · 1 comment · Fixed by #27696
Closed
4 tasks

[Issue] Initialize inline translations module only when they are enabled #29553

m2-assistant bot opened this issue Aug 14, 2020 · 1 comment · Fixed by #27696
Assignees
@krzksz
Labels
Component: Translation Fixed in 2.4.x The issue has been fixed in 2.4-develop branch Issue: Format is not valid Gate 1 Failed. Automatic verification of issue format is failed Priority: P2 A defect with this priority could have functionality issues which are not to expectations. Progress: done Reported on 2.4.0 Indicates original Magento version for the Issue report. Severity: S3 Affects non-critical data or functionality and does not force users to employ a workaround.
Projects
High Priority Backlog

Comments

@m2-assistant
Copy link

m2-assistant bot commented Aug 14, 2020

This issue is automatically created based on existing pull request: #27696: Initialize inline translations module only when they are enabled

Description (*)

This PR is a part of my effort to improve Magento 2 frontend performance.

The focus point of this PR was to remove downloading of inline translations JavaScript modules when they are not enabled. This was happening because they were defined in deps, which is completely redundant because they are properly initialized in https://github.com/krzksz/magento2/blob/98e04e174906799b941011a9fb1b5890a5e7cae9/app/code/Magento/Translation/view/frontend/templates/translate_inline.phtml#L57

While I was touching the files anyway I took the liberty to:

  • Move escapeHTML to utilities (it is not used anywhere else and there is already an unescape counterpart).
  • Fixed utilities dependency for serialize method.
  • Simplify logic and variables in few places.

Performance improvement

Tested on Magento 2.4-develop with Luma and sample data.

  • 207 → 203 JS requests (-1.93%).
  • 794.39kB → 773.44kB JS transferred (-2.64%).

Manual testing scenarios (*)

  1. When inline translations are disabled there is no request for translate-inline JS file.
  2. When inline translations are enabled, they work as expected.

Questions or comments

Contribution checklist (*)

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All new or changed code is covered with unit/integration tests (if applicable)
  • All automated tests passed successfully (all builds are green)
@krzksz krzksz mentioned this issue Aug 14, 2020
Merged
5 tasks
@ghost ghost assigned krzksz Aug 14, 2020
@ghost ghost added this to Ready for QA in Community Backlog Aug 14, 2020
@magento-engcom-team magento-engcom-team added the Issue: Format is not valid Gate 1 Failed. Automatic verification of issue format is failed label Aug 14, 2020
@ghost ghost moved this from Ready for QA to PR In Progress in Community Backlog Aug 14, 2020
@ghost ghost added Progress: PR in progress Priority: P2 A defect with this priority could have functionality issues which are not to expectations. Severity: S3 Affects non-critical data or functionality and does not force users to employ a workaround. and removed Progress: ready for QA labels Aug 14, 2020
@magento-engcom-team magento-engcom-team added the Fixed in 2.4.x The issue has been fixed in 2.4-develop branch label Aug 26, 2020
@magento-engcom-team
Copy link
Contributor

Hi @m2-assistant[bot]. Thank you for your report.
The issue has been fixed in #27696 by @krzksz in 2.4-develop branch
Related commit(s):

The fix will be available with the upcoming 2.4.2 release.

@ghost ghost moved this from PR In Progress to Done (last 30 days) in Community Backlog Aug 26, 2020
@magento-engcom-team magento-engcom-team added the Reported on 2.4.0 Indicates original Magento version for the Issue report. label Nov 13, 2020
@m2-community-project m2-community-project bot removed this from Done (last 30 days) in Community Backlog Nov 13, 2020
@sdzhepa sdzhepa mentioned this issue May 10, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@krzksz krzksz

Labels
Component: Translation Fixed in 2.4.x The issue has been fixed in 2.4-develop branch Issue: Format is not valid Gate 1 Failed. Automatic verification of issue format is failed Priority: P2 A defect with this priority could have functionality issues which are not to expectations. Progress: done Reported on 2.4.0 Indicates original Magento version for the Issue report. Severity: S3 Affects non-critical data or functionality and does not force users to employ a workaround.
Projects
High Priority Backlog
  
Done
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Initialize inline translations module only when they are enabled krzksz/magento2
2 participants
@krzksz @magento-engcom-team