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.

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

Minicart-search-logo-not-vertically-aligned #20955

Conversation

amol2jcommerce
Copy link
Contributor

Description (*)

Fixed Issues (if relevant)

  1. Minicart, search & logo not vertically aligned between 640 to767 device pixel. #20905: Minicart, search & logo not vertically aligned between 640 to767 device pixel.

Manual testing scenarios (*)

1..Go to frontend -> resize screen (min width 640 and max width 767)

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)

Nainesh and others added 5 commits January 23, 2019 02:59
… input field are short width on tablet view
[Forwardport] 'Fixes-for-customer-login-page-input-field' :: On customer login page…
… input field are short width on tablet view
[Forwardport] 'Fixes-for-customer-login-page-input-field' :: On customer login page…
@magento-engcom-team
Copy link
Contributor

Hi @amol2jcommerce. 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

@cedarvinda
Copy link
Member

@magento-engcom-team give me test instance.

@magento-engcom-team
Copy link
Contributor

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

@magento-engcom-team
Copy link
Contributor

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

@torhoehn torhoehn self-requested a review February 4, 2019 10:18
@torhoehn torhoehn self-assigned this Feb 4, 2019
@magento-engcom-team
Copy link
Contributor

Hi @torhoehn, thank you for the review.
ENGCOM-4118 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-20955.instances.magento-community.engineering/admin
Login: admin Password: 123123q

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.

@amol2jcommerce @torhoehn
I don't agree with these changes.
Code should follow coding standards and each component change should be placed into correct module design file.
For correct fix there are really no need for extra lines of code, just breakpoint for design should be changed from @screen__s to @screen__m
example in:

.media-width(@extremum, @break) when (@extremum = 'max') and (@break = @screen__s) {

.media-width(@extremum, @break) when (@extremum = 'max') and (@break = @screen__s) {

and
.media-width(@extremum, @break) when (@extremum = 'max') and (@break = @screen__s) {

Copy link
Contributor

@torhoehn torhoehn left a comment

Choose a reason for hiding this comment

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

@Karlasa Ah, you're right.

@amol2jcommerce
Copy link
Contributor Author

amol2jcommerce commented Feb 11, 2019

Hi @Karlasa @torhoehn changes updated as per requested.

@magento-engcom-team
Copy link
Contributor

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

@magento-engcom-team
Copy link
Contributor

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

@VasylShvorak
Copy link
Contributor

✔️ QA passed

Copy link
Member

@sivaschenko sivaschenko left a comment

Choose a reason for hiding this comment

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

Hi @amol2jcommerce the commits in this pull request are not linked to your github account. Can you please check the email configured for github and push the changes as a single commit correctly linked to the author.

@amol2jcommerce
Copy link
Contributor Author

Hi @sivaschenko, I closed this PR due to committed from different account and that account didn't sign CLA. We generate New PR #21638 for this & sign CLA account.

@ghost
Copy link

ghost commented Mar 8, 2019

Hi @amol2jcommerce, 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.

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.

10 participants