Skip to content

magento/magento2#23481: Billing/Shipping Address edit form design update from order backend #25699

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
Merged
merged 1 commit into from
Dec 12, 2019
Merged

magento/magento2#23481: Billing/Shipping Address edit form design update from order backend #25699

merged 1 commit into from
Dec 12, 2019

Conversation

alexey-rakitin
Copy link
Contributor

@alexey-rakitin alexey-rakitin commented Nov 22, 2019 •
edited by krzksz
Loading

Description (*)

  • Form wrapper html tag changed from 'fieldset' to 'div' to avoid standard fieldset tag borders
  • Added 'admin__fieldset' class to form for standard admin styles to work properly

Fixed Issues (if relevant)

  1. Billing/Shipping Address edit form design update from order backend #23481: Billing/Shipping Address edit form design update from order backend
  2. Sales order Address Information edit form layout design improvement. #19805: Sales order Address Information edit form layout design improvement.

Manual testing scenarios (*)

  1. Go to Admin -> Sales -> Invoices -> Click on View -> Address Information section -> Billing Address/Shipping Address
  2. Click on Edit
  3. Address form should look according to Magento 2 Admin form pattern library (labels to the left of inputs)

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)

@m2-assistant
Copy link

m2-assistant bot commented Nov 22, 2019

Hi @alexey-rakitin. 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 give me test instance - deploy test instance based on PR changes
  • @magento give me 2.3-develop instance - deploy vanilla Magento instance

For more details, please, review the Magento Contributor Guide documentation.

krzksz
krzksz suggested changes Nov 22, 2019
View reviewed changes
Copy link
Contributor

@krzksz krzksz left a comment

Choose a reason for hiding this comment

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

This PR duplicates work done within #25548

@ghost ghost assigned krzksz Nov 22, 2019
@krzksz
Copy link
Contributor

krzksz commented Nov 24, 2019

Hey @alexey-rakitin, after reviewing your changes I'm actually leaning towards your solution to this styling problem but let me compare the looks of the forms first.

@krzksz
Copy link
Contributor

krzksz commented Nov 24, 2019

@magento give me test instance

@magento-engcom-team
Copy link
Contributor

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

@magento-engcom-team
Copy link
Contributor

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

krzksz
krzksz suggested changes Nov 24, 2019
View reviewed changes
Copy link
Contributor

@krzksz krzksz left a comment

Choose a reason for hiding this comment

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

Everything looks good, but the margin between notice and form title is missing:
obraz

@krzksz krzksz mentioned this pull request Nov 24, 2019
Closed
4 tasks
@alexey-rakitin
#23481: Billing/Shipping Address edit form design update from order b…
95eea52 
…ackend

- Messages container added to correctly apply margin between message and form
- Form wrapper html tag changed from 'fieldset' to 'div'
- Added 'admin__fieldset' class to form for standard admin styles to work properly
@alexey-rakitin
Copy link
Contributor Author

Hi, @krzksz, please check amended commit. I added messages container, now margin is ok.

@krzksz krzksz added the Auto-Tests: Not Required Changes in Pull Request does not require coverage by auto-tests label Nov 25, 2019
@magento-engcom-team
Copy link
Contributor

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

@engcom-Alfa engcom-Alfa self-assigned this Nov 25, 2019
@engcom-Alfa
Copy link
Contributor

✔️ QA Passed

@engcom-Foxtrot engcom-Foxtrot self-assigned this Nov 26, 2019
@engcom-Foxtrot engcom-Foxtrot removed the Auto-Tests: Not Required Changes in Pull Request does not require coverage by auto-tests label Nov 26, 2019
@engcom-Foxtrot engcom-Foxtrot added the Auto-Tests: Not Covered Changes in Pull Request requires coverage by auto-tests label Nov 26, 2019
@engcom-Foxtrot
Copy link
Contributor

Hello, @alexey-rakitin. Thank you for contribution. Unfortunately, we cannot merge your PR until changes are covered with tests.

@alexey-rakitin
Copy link
Contributor Author

Hello, @engcom-Foxtrot . With which tests should I cover this PR? MFTF?

@sidolov sidolov changed the base branch from 2.3-develop to 2.4-develop December 5, 2019 17:12
@VladimirZaets VladimirZaets added Auto-Tests: Not Required Changes in Pull Request does not require coverage by auto-tests and removed Auto-Tests: Not Covered Changes in Pull Request requires coverage by auto-tests labels Dec 9, 2019
@ghost ghost assigned VladimirZaets Dec 9, 2019
@engcom-Alfa
Copy link
Contributor

✔️ QA Passed

@engcom-Golf engcom-Golf self-assigned this Dec 10, 2019
This was referenced Dec 12, 2019
Closed
Closed
magento-engcom-team pushed a commit that referenced this pull request Dec 12, 2019
@VladimirZaets
ENGCOM-6328: #23481: Billing/Shipping Address edit form design update…
f0be18b 
… from order backend #25699
@magento-engcom-team magento-engcom-team merged commit 95eea52 into magento:2.4-develop Dec 12, 2019
@m2-assistant
Copy link

m2-assistant bot commented Dec 12, 2019

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

@sdzhepa sdzhepa mentioned this pull request May 9, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@krzksz krzksz krzksz approved these changes

@vsevolod-tarasov vsevolod-tarasov vsevolod-tarasov approved these changes

@VladimirZaets VladimirZaets VladimirZaets approved these changes

@YevSent YevSent Awaiting requested review from YevSent

Assignees

@krzksz krzksz

@VladimirZaets VladimirZaets

@engcom-Alfa engcom-Alfa

@engcom-Foxtrot engcom-Foxtrot

@engcom-Golf engcom-Golf

Labels
Area: Frontend Auto-Tests: Not Required Changes in Pull Request does not require coverage by auto-tests Component: Sales Progress: accept Release Line: 2.4
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@alexey-rakitin @krzksz @magento-engcom-team @engcom-Alfa @engcom-Foxtrot @vsevolod-tarasov @VladimirZaets @sidolov @engcom-Golf