Remove unnecessary scripts review summary #34643

in-session · 2021-11-15T20:15:05Z

I don't see the need to use a script for the rating here I think with inline style that should be enough. This optimization avoids loading and executing JavaScript, which can result in faster page load time. In addition, the code is simplified and more readable because the CSS styles are specified directly in the HTML markup.

On women/tops-women/jackets-women.html removes 12 script tags

Before:

After:

Resolved issues:

resolves [Issue] Remove unnecessary scripts review summary #37776: Remove unnecessary scripts review summary

m2-assistant · 2021-11-15T20:15:07Z

Hi @in-session. Thank you for your contribution
Here are some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

@magento give me test instance - deploy test instance based on PR changes
@magento give me 2.4-develop instance - deploy vanilla Magento instance

❗ Automated tests can be triggered manually with an appropriate comment:

@magento run all tests - run or re-run all required tests against the PR changes
@magento run <test-build(s)> - run or re-run specific test build(s)
For example: @magento run Unit Tests

<test-build(s)> is a comma-separated list of build names. Allowed build names are:

Database Compare
Functional Tests CE
Functional Tests EE,
Functional Tests B2B
Integration Tests
Magento Health Index
Sample Data Tests CE
Sample Data Tests EE
Sample Data Tests B2B
Static Tests
Unit Tests
WebAPI Tests
Semantic Version Checker

You can find more information about the builds here

ℹ️ Run only required test builds during development. Run all test builds before sending your pull request for review.

For more details, review the Magento Contributor Guide documentation.

⚠️ According to the Magento Contribution requirements, all Pull Requests must go through the Community Contributions Triage process. Community Contributions Triage is a public meeting.

🕙 You can find the schedule on the Magento Community Calendar page.

📞 The triage of Pull Requests happens in the queue order. If you want to speed up the delivery of your contribution, join the Community Contributions Triage session to discuss the appropriate ticket.

🎥 You can find the recording of the previous Community Contributions Triage on the Magento Youtube Channel

✏️ Feel free to post questions/proposals/feedback related to the Community Contributions Triage process to the corresponding Slack Channel

in-session · 2021-11-15T20:36:45Z

@magento run all tests

magento-automated-testing · 2021-11-15T20:36:53Z

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

mrtuvn · 2021-11-15T23:23:36Z

reason because it's rely on id . I think that why reason author use that approach
d2745c5#diff-b2e9bb45b6102e1c93288b1f9b3a7e3a0a1c0da52296a441c0b805e32b0e8a58

in-session · 2021-11-16T07:25:41Z

@magento run all tests

magento-automated-testing · 2021-11-16T07:25:49Z

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

ihor-sviziev · 2021-11-18T08:54:57Z

@sidolov, could you provide some info on why this block was added in MC-29420: Remove event handlers from CE 70ede03?

sidolov · 2021-11-18T16:15:34Z

Hi @ihor-sviziev , it was done in the scope of the story: "Move inline event handlers, styles to dedicated tags in phtml templates"

ihor-sviziev · 2021-11-18T16:27:03Z

Hi @in-session,
It looks like it's needed for the CSP.
Here is more details: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/style-src#violation_cases

in-session · 2021-11-18T16:39:44Z

Hi @ihor-sviziev @sidolov

Sorry, I don't quite get it yet :-)

Due to the changes in the files, a script is no longer needed to display the rating summary stars.
Or does the problem only relate to the view.phtml?

ihor-sviziev · 2021-11-18T16:43:57Z

@in-session ifi got it correctly, the CSP rules in Magento disallows using inline styles, that's why it was added

andrewbess

Hello @in-session
Thank you for your contribution.
Could you please fix static tests
Thank you in advance.

in-session · 2023-07-17T10:40:44Z

@magento run all tests

magento-automated-testing · 2023-07-17T10:40:52Z

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please message the #magento-devops slack channel if they don't show in a reasonable amount of time and a representative will look into any issues.

engcom-Lima · 2023-07-19T11:46:06Z

@magento run all tests

magento-automated-testing · 2023-07-19T11:46:12Z

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please message the #magento-devops slack channel if they don't show in a reasonable amount of time and a representative will look into any issues.

engcom-Lima · 2023-07-20T10:57:47Z

@magento give me 2.4-develop instance

magento-deployment-service · 2023-07-20T10:57:50Z

Hi @engcom-Lima. Thank you for your request. I'm working on Magento instance for you.

magento-deployment-service · 2023-07-20T11:02:49Z

Hi @engcom-Lima, here is your Magento Instance: https://02a2be646aebdb55cff8ce5523c383e1.instances-prod.magento-community.engineering
Admin access: https://02a2be646aebdb55cff8ce5523c383e1.instances-prod.magento-community.engineering/admin_98a9
Login: ad0afc25
Password: f5984c8c738e

engcom-Lima · 2023-07-20T11:09:37Z

@magento give me test instance

magento-deployment-service · 2023-07-20T11:09:40Z

Hi @engcom-Lima. Thank you for your request. I'm working on Magento instance for you.

magento-deployment-service · 2023-07-20T11:15:32Z

Hi @engcom-Lima, here is your Magento Instance: https://39fd41f4a9b1c3675af301a185ecf248.instances-prod.magento-community.engineering
Admin access: https://39fd41f4a9b1c3675af301a185ecf248.instances-prod.magento-community.engineering/admin_1b15
Login: 5334506e
Password: 2ffc2582b668

engcom-Lima · 2023-07-20T12:29:46Z

✔️ QA Passed

Preconditions:

Install fresh Magento 2.4-develop and PHP 8.1

Manual testing scenario:

Disable the FPC in the backend and reload the category page.
Verify whether the script got removed or not.

Before: ✖️

After: ✔️

Additionally tested:
Verified the performance of page loading.

No additional manual test cases are required as part of regression as this PR is related to performance improvement.

Builds are failed. Hence, moving this PR to Extended Testing.

engcom-Lima · 2023-07-20T12:30:09Z

@magento create issue

engcom-Charlie · 2024-10-08T09:07:28Z

@magento run all tests

engcom-Charlie · 2024-10-08T11:21:29Z

@magento run Functional Tests CE, Integration Tests, Static Tests

engcom-Charlie · 2024-10-09T08:24:04Z

@magento run Functional Tests B2B, Static Tests

engcom-Charlie · 2024-10-09T09:15:27Z

@magento run all tests

engcom-Charlie · 2024-10-09T17:26:40Z

@magento run Functional Tests B2B, Functional Tests EE

engcom-Charlie · 2024-10-10T06:45:08Z

The functional B2B and EE tests failures in recent 2 builds are not consistent. The tests are not part of the PR or not failing because of the PR changes, considering them as flaky. Hence moving this PR to Merge in Progress.

Functional B2B Run 1:
https://public-results-storage-prod.magento-testing-service.engineering/reports/magento/magento2/pull/34643/68daabc9e9d8a3d8719cd5c8fa5146bb/Functional/allure-report-b2b/index.html#categories

Run 2
https://public-results-storage-prod.magento-testing-service.engineering/reports/magento/magento2/pull/34643/d0715457acdbfd37ef6b5242b60b9b15/Functional/allure-report-b2b/index.html#categories

Functional EE Run 1:
https://public-results-storage-prod.magento-testing-service.engineering/reports/magento/magento2/pull/34643/825f1663cc7d0bfc669e2e20bad92687/Functional/allure-report-ee/index.html#categories

Run 2:
https://public-results-storage-prod.magento-testing-service.engineering/reports/magento/magento2/pull/34643/da732050ff9b0397f4ad58e35896b02c/Functional/allure-report-ee/index.html#categories

Remove unnecessary scripts

e8a310c

magento-engcom-team added Area: Frontend Component: Review Release Line: 2.4 labels Nov 15, 2021

in-session changed the title ~~Remove unnecessary scripts~~ Remove unnecessary scripts review summary Nov 15, 2021

m2-community-project bot added the Progress: pending review label Nov 15, 2021

in-session added 3 commits November 15, 2021 21:20

Remove unnecessary scripts

6225ed8

Update list.phtml

41ba1a6

Update view.phtml

3850c71

in-session added 2 commits November 16, 2021 08:05

Update list.phtml

2a28382

Update view.phtml

90dfb7b

ihor-sviziev self-assigned this Nov 18, 2021

m2-community-project bot added Progress: review and removed Progress: pending review labels Nov 18, 2021

sidolov added the Priority: P3 May be fixed according to the position in the backlog. label Nov 18, 2021

andrewbess self-assigned this Nov 18, 2021

andrewbess requested changes Nov 18, 2021

View reviewed changes

m2-community-project bot added Progress: needs update and removed Progress: review labels Nov 18, 2021

engcom-Lima self-assigned this Jul 19, 2023

m2-community-project bot added Progress: testing in progress and removed Progress: ready for testing labels Jul 19, 2023

m2-assistant bot mentioned this pull request Jul 20, 2023

[Issue] Remove unnecessary scripts review summary #37776

Closed

m2-community-project bot added Progress: ready for testing and removed Progress: testing in progress labels Jul 20, 2023

engcom-Charlie self-assigned this Oct 3, 2024

Merge branch '2.4-develop' into patch-12

5e901d4

Fixed static test failures

4959aa5

Priyakshic added the Project: Community Picked PRs upvoted by the community label Oct 15, 2024

magento-devops-reposync-svc merged commit 015206d into magento:2.4-develop Oct 18, 2024
10 of 12 checks passed

in-session deleted the patch-12 branch October 28, 2024 05:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Remove unnecessary scripts review summary #34643

Remove unnecessary scripts review summary #34643

Uh oh!

in-session commented Nov 15, 2021 •

edited by m2-assistant bot

Loading

m2-assistant bot commented Nov 15, 2021

in-session commented Nov 15, 2021

magento-automated-testing bot commented Nov 15, 2021

mrtuvn commented Nov 15, 2021

in-session commented Nov 16, 2021

magento-automated-testing bot commented Nov 16, 2021

ihor-sviziev commented Nov 18, 2021

sidolov commented Nov 18, 2021

ihor-sviziev commented Nov 18, 2021

in-session commented Nov 18, 2021 •

edited

Loading

ihor-sviziev commented Nov 18, 2021

andrewbess left a comment

in-session commented Jul 17, 2023

magento-automated-testing bot commented Jul 17, 2023

engcom-Lima commented Jul 19, 2023

magento-automated-testing bot commented Jul 19, 2023

engcom-Lima commented Jul 20, 2023

magento-deployment-service bot commented Jul 20, 2023

magento-deployment-service bot commented Jul 20, 2023

engcom-Lima commented Jul 20, 2023

magento-deployment-service bot commented Jul 20, 2023

magento-deployment-service bot commented Jul 20, 2023

engcom-Lima commented Jul 20, 2023 •

edited

Loading

engcom-Lima commented Jul 20, 2023

engcom-Charlie commented Oct 8, 2024

engcom-Charlie commented Oct 8, 2024

engcom-Charlie commented Oct 9, 2024

engcom-Charlie commented Oct 9, 2024

engcom-Charlie commented Oct 9, 2024

engcom-Charlie commented Oct 10, 2024

Uh oh!

Remove unnecessary scripts review summary #34643

Remove unnecessary scripts review summary #34643

Uh oh!

Conversation

in-session commented Nov 15, 2021 • edited by m2-assistant bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Resolved issues:

m2-assistant bot commented Nov 15, 2021

in-session commented Nov 15, 2021

magento-automated-testing bot commented Nov 15, 2021

mrtuvn commented Nov 15, 2021

in-session commented Nov 16, 2021

magento-automated-testing bot commented Nov 16, 2021

ihor-sviziev commented Nov 18, 2021

sidolov commented Nov 18, 2021

ihor-sviziev commented Nov 18, 2021

in-session commented Nov 18, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

ihor-sviziev commented Nov 18, 2021

andrewbess left a comment

Choose a reason for hiding this comment

in-session commented Jul 17, 2023

magento-automated-testing bot commented Jul 17, 2023

engcom-Lima commented Jul 19, 2023

magento-automated-testing bot commented Jul 19, 2023

engcom-Lima commented Jul 20, 2023

magento-deployment-service bot commented Jul 20, 2023

magento-deployment-service bot commented Jul 20, 2023

engcom-Lima commented Jul 20, 2023

magento-deployment-service bot commented Jul 20, 2023

magento-deployment-service bot commented Jul 20, 2023

engcom-Lima commented Jul 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✔️ QA Passed

engcom-Lima commented Jul 20, 2023

engcom-Charlie commented Oct 8, 2024

engcom-Charlie commented Oct 8, 2024

engcom-Charlie commented Oct 9, 2024

engcom-Charlie commented Oct 9, 2024

engcom-Charlie commented Oct 9, 2024

engcom-Charlie commented Oct 10, 2024

Uh oh!

in-session commented Nov 15, 2021 •

edited by m2-assistant bot

Loading

in-session commented Nov 18, 2021 •

edited

Loading

engcom-Lima commented Jul 20, 2023 •

edited

Loading