Skip to content

Conversation

@LuciferStrome
Copy link
Member

@LuciferStrome LuciferStrome commented Dec 14, 2018

Typo in bootstrap.js file

Description (*)

Typo Corrected

Fixed Issues (if relevant)

Typo in bootstrap.js

Manual testing scenarios (*)

Not Needed

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 on Travis CI are green)

Typo in bootstrap.js file
@magento-cicd2
Copy link
Contributor

magento-cicd2 commented Dec 14, 2018

CLA assistant check
All committers have signed the CLA.

@magento-engcom-team
Copy link
Contributor

Hi @LuciferStrome. Thank you for your contribution
Here is 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-engcom-team give me test instance - deploy test instance based on PR changes
  • @magento-engcom-team give me 2.3-develop instance - deploy vanilla Magento instance

For more details, please, review the Magento Contributor Assistant documentation

Copy link
Member

@milindsingh milindsingh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will create issues everywhere the index is used.

@VladimirZaets
Copy link
Contributor

Hi @LuciferStrome. Thanks for collaboration.
@milindsingh Yes, you are right. Currently, I found usage only correct afterRender in Magento. KnockoutJS that is used as a core of the bindings has the native afterRender method that currently uses in all places. With this fix, we will override the native KnockoutJS binding.

I looked the code of our custom afterRender binding and looks like it works like original afterRender and only passes additional param. I think, we should check manually this case

@LuciferStrome
Copy link
Member Author

Hi @VladimirZaets , Have you or someone from your team checked this case manually?

@orlangur orlangur removed their request for review December 17, 2018 10:09
@VladimirZaets
Copy link
Contributor

Hi @LuciferStrome. We already check this case. I was right and our custom afterRender implementation works as native.

@magento-engcom-team
Copy link
Contributor

Hi @VladimirZaets, thank you for the review.
ENGCOM-3719 has been created to process this Pull Request

@magento-engcom-team
Copy link
Contributor

@LuciferStrome thank you for contributing. Please accept Community Contributors team invitation here to gain extended permissions for this repository.

@magento-engcom-team magento-engcom-team merged commit 2c87a21 into magento:2.3-develop Dec 22, 2018
magento-engcom-team pushed a commit that referenced this pull request Dec 22, 2018
@magento-engcom-team
Copy link
Contributor

Hi @LuciferStrome. Thank you for your contribution.
We will aim to release these changes as part of 2.3.1.
Please check the release notes for final confirmation.

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.

5 participants