-
-
Notifications
You must be signed in to change notification settings - Fork 449
Adminhtml: improved info for order sent status #4623
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
base: main
Are you sure you want to change the base?
Conversation
This PR needs changes before merging. |
For change wordings please make suggestions via review (select lines & ctrl+h). |
<td class="value"><strong><span id="order_status"><?php echo $_order->getStatusLabel() ?></span></strong></td> | ||
</tr> | ||
<tr> | ||
<td class="label"><label><?php echo Mage::helper('sales')->__('Order Confirmation Email') ?></label></td> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer a two-word label. These two variants are more appropriate.
Variant 1
<td class="label"><label><?php echo Mage::helper('sales')->__('Order Confirmation Email') ?></label></td> | |
<td class="label"><label><?php echo Mage::helper('sales')->__('Email Confirmation') ?></label></td> |
Variant 2
<td class="label"><label><?php echo Mage::helper('sales')->__('Order Confirmation Email') ?></label></td> | |
<td class="label"><label><?php echo Mage::helper('sales')->__('Order Confirmation') ?></label></td> |
I asked Copilot and Chatgpt for their opinions, both argued that "Order Confirmation" is more appropriate, as it covers confirmations sent by email, SMS, phone. Personally, I would prefer Email Confirmation because it specifies the confirmation method available in OpenMage.
If we are analyzing the label names in that block that obviously refer to the order, then the word "Order" should be removed from the "Order Date" and "Order Status" forms. Here is what it would look like in my opinion
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I disagree on that.
- "Email" suffix ... its no SMS, fax, ...
- "Order" prefix ... this block is displayed for orders, invoices, shippings ... so it could be missleading w/o it.
I really thought about how to name it - and it was also named "order confirmation email" before.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This pull request improves the visibility and presentation of the order confirmation email status in the admin interface by moving it from the order header to a dedicated row in the order information table and updating the visual styling of copy icons.
- Moves email sent status from order header to dedicated table row with clearer labels
- Updates CSS styling to float copy icons to the right for better visual alignment
- Refactors code for better readability by removing inline conditional logic
Reviewed Changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
app/design/adminhtml/default/default/template/sales/order/view/info.phtml | Relocates email status from header to table row, adds new translation strings, improves code formatting |
app/locale/en_US/Mage_Sales.csv | Adds translation strings for "Order Confirmation Email", "Sent", and "Not sent" |
skin/adminhtml/default/default/boxes.css | Updates copy icon styling with better margins and right float positioning |
skin/adminhtml/default/openmage/override.css | Adds right float positioning for copy icons |
skin/adminhtml/default/openmage/scss/override.scss | Adds right float positioning for copy icons in SCSS source |
app/design/adminhtml/default/default/template/sales/order/view/info.phtml
Outdated
Show resolved
Hide resolved
…/info.phtml Co-authored-by: Copilot <[email protected]>
Please retry analysis of this Pull-Request directly on SonarQube Cloud |
Description (*)
See #4620 (comment)
Fixed Issues (if relevant)