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

Added space above error message. #21074

Merged
merged 4 commits into from
Mar 14, 2019
Merged

Added space above error message. #21074

merged 4 commits into from
Mar 14, 2019

Conversation

suryakant-krish
Copy link
Contributor

@suryakant-krish suryakant-krish commented Feb 8, 2019 •
edited
Loading

Description (*)

  1. This is direct PR because it is small change.
  2. Area Frontend. when user group product add to cart without add quantity error message show.

Fixed Issues (if relevant)

  1. This is direct PR.
  2. Go to frontend > Find any group product Ex "Set of Sprite Yoga Straps". > on product page click on add to cart button error message show.
  3. Reproduce on Magento 2.3

Manual testing scenarios (*)

  1. Actual Result.
    actual-result

  2. Expected Result.
    expected-result

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)

@magento-engcom-team
Copy link
Contributor

Hi @suryakant-krish. 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

@dmytro-ch dmytro-ch self-assigned this Feb 8, 2019
@magento-engcom-team
Copy link
Contributor

Hi @dmytro-ch, thank you for the review.
ENGCOM-4181 has been created to process this Pull Request

@Karlasa
Copy link
Contributor

Karlasa commented Feb 8, 2019

@magento-engcom-team give me test instance

@magento-engcom-team
Copy link
Contributor

Hi @Karlasa. Thank you for your request. I'm working on Magento instance for you

@magento-engcom-team
Copy link
Contributor

Hi @Karlasa, here is your new Magento instance.
Admin access: https://pr-21074.instances.magento-community.engineering/admin
Login: admin Password: 123123q

Karlasa
Karlasa suggested changes Feb 8, 2019
View reviewed changes
Copy link
Contributor

@Karlasa Karlasa left a comment

Choose a reason for hiding this comment

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

@suryakant-krish @dmytro-ch
This fix looks bit too general and will affect a lot of other places that has this class.
maybe add margin into #validation-message-box and under grouped product module?

@dmytro-ch
Copy link
Contributor

dmytro-ch commented Feb 8, 2019 •
edited
Loading

@Karlasa, thank you for the update.

I was also thinking about that but decided that having kind of base 10px space above this block allows us to make sure there are no additional issues with other blocks inserted before the product.info.addtocart block.
Also, there are other rules overriding this value for other product types, e.g. 20px for configurable products etc.

But yes, I agree that it makes sense to make the changes for grouped products only for this particular case in order to avoid some unexpected behavior.

Thank you!

@suryakant-krish
Copy link
Contributor Author

@dmytro-ch and @Karlasa

I have updated code only for group product used page layout handle hierarchy.

Please review this and let me know.

@sivaschenko
Copy link
Member

@magento-engcom-team give me test instance

@magento-engcom-team
Copy link
Contributor

Hi @sivaschenko. Thank you for your request. I'm working on Magento instance for you

@magento-engcom-team
Copy link
Contributor

Hi @sivaschenko, here is your new Magento instance.
Admin access: https://pr-21074.instances.magento-community.engineering/admin
Login: admin Password: 123123q

Karlasa
Karlasa suggested changes Feb 18, 2019
View reviewed changes
app/design/frontend/Magento/luma/Magento_Catalog/web/css/source/_module.less Outdated
@@ -293,6 +293,12 @@
}
}

.page-product-grouped {
Copy link
Contributor

Choose a reason for hiding this comment

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

Grouped product specific code should be placed under right module Magento__GroupedProduct in this case.

@suryakant-krish
Copy link
Contributor Author

@Karlasa,

I have moved code under "Magento__GroupedProduct".

Karlasa
Karlasa reviewed Mar 4, 2019
View reviewed changes
app/design/frontend/Magento/luma/Magento_Catalog/web/css/source/_module.less Outdated
@@ -293,6 +293,8 @@
}
}


Copy link
Contributor

Choose a reason for hiding this comment

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

remove these 2 added lines :)

@Karlasa
Copy link
Contributor

Karlasa commented Mar 4, 2019

@suryakant-krish
Thanks, just a bit more code clean up and I'm happy 👍

@suryakant-krish
Copy link
Contributor Author

@Karlasa,

I have updated code as you have mentioned.

@ghost ghost assigned Karlasa Mar 4, 2019
@magento-engcom-team
Copy link
Contributor

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

@VasylShvorak VasylShvorak self-assigned this Mar 7, 2019
@VasylShvorak
Copy link
Contributor

✔️ QA passed

@p-bystritsky p-bystritsky self-assigned this Mar 7, 2019
@magento-engcom-team magento-engcom-team merged commit 64f270a into magento:2.3-develop Mar 14, 2019
@ghost
Copy link

ghost commented Mar 14, 2019

Hi @suryakant-krish, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

magento-engcom-team pushed a commit that referenced this pull request Mar 14, 2019
@sidolov
ENGCOM-4181: Added space above error message. #21074
6a74f09
@magento-engcom-team magento-engcom-team added this to the Release: 2.3.2 milestone Mar 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dmytro-ch dmytro-ch dmytro-ch approved these changes

@Karlasa Karlasa Karlasa approved these changes

@zanilee zanilee Awaiting requested review from zanilee

Assignees

@Karlasa Karlasa

@dmytro-ch dmytro-ch

@p-bystritsky p-bystritsky

@VasylShvorak VasylShvorak

Labels
Area: Design/Frontend Cleanup Partner: Krish TechnoLabs partners-contribution Pull Request is created by Magento Partner Progress: accept Release Line: 2.3
Projects
None yet
Milestone
Release: 2.3.2
Development

Successfully merging this pull request may close these issues.
7 participants
@suryakant-krish @magento-engcom-team @Karlasa @dmytro-ch @sivaschenko @VasylShvorak @p-bystritsky