OpenMage / magento-lts

Official OpenMage LTS codebase | Migrate easily from Magento Community Edition in minutes! Download the source code for free or contribute to OpenMage LTS | Security vulnerability patches, bug fixes, performance improvements and more.
https://www.openmage.org
Open Software License 3.0
870 stars 436 forks source link

Backend: Improved "copy button" style in legacy admin theme #4072

Closed ma4nn closed 4 months ago

ma4nn commented 4 months ago

Description (*)

With the latest addition of the copy button feature, the order header in the legacy admin theme looks a bit broken: Bildschirmfoto 2024-07-03 um 08 59 10

This PR improves the styling of the button in that section: Bildschirmfoto 2024-07-03 um 08 58 31

Related Pull Requests

Fixed Issues (if relevant)

Manual testing scenarios (*)

  1. Copy button looks nice in legacy and openmage admin theme
  2. Copy button functionality is still present

Questions or comments

Contribution checklist (*)

addison74 commented 4 months ago

I just wanted to make an observation on this topic because I saw the change with the new v20 version.

In CSS you should your position: absolute then you can arrange the icon into the right position. This is a test for the email icon

.icon-copy {
    display: inline-block;
    background-image: url('data:image/svg+xml,<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 111.07 122.88"><path d="M97.67 20.81h0l.01.02a13.38 13.38 0 0 1 9.46 3.93 13.32 13.32 0 0 1 3.9 9.42h.02v.02 75.28.01h-.02c-.01 3.68-1.51 7.03-3.93 9.46a13.32 13.32 0 0 1-9.42 3.9v.02h-.02-59.19-.01v-.02c-3.69-.01-7.04-1.5-9.46-3.93a13.37 13.37 0 0 1-3.91-9.42h0V34.2v-.01h.02c.01-3.69 1.52-7.04 3.94-9.46 2.41-2.4 5.73-3.9 9.42-3.91v-.02h.02l59.17.01h0zM.02 75.38L0 13.39v-.01h.02a13.44 13.44 0 0 1 3.93-9.46A13.37 13.37 0 0 1 13.37.01V0h.02 59.19c7.69 0 8.9 9.96.01 10.16H13.4h-.02v-.02c-.88 0-1.68.37-2.27.97-.59.58-.96 1.4-.96 2.27h.02v.01 3.17 58.81c0 8.26-10.15 8.72-10.15.01h0zm100.89 34.11V34.2v-.02h.02c0-.87-.37-1.68-.97-2.27-.59-.58-1.4-.96-2.28-.96v.02h-.01-59.19-.02v-.02c-.88 0-1.68.38-2.27.97-.59.58-.96 1.4-.96 2.27h.02v.01 75.28.02h-.02c0 .88.38 1.68.97 2.27s1.4.96 2.27.96v-.02h.01 59.19.02v.02c.87 0 1.68-.38 2.27-.97.59-.58.96-1.4.96-2.27h-.01 0 0 0z" fill-rule="evenodd"/></svg>');
    background-repeat: no-repeat;
    background-size: contain;
    cursor: pointer;
    width: 12px;
    height: 12px;
    margin: 3px 0 0 6px;
    position: absolute;

and the same width, height and margin for the order icon. In this way both icons will look good. The color for the order must be the same with text. For the email it should be evaluate orange from the email or black from the text.

ma4nn commented 4 months ago

@ADDISON74 I would not recommend using position: absolute, especially without any relative container outside. Your proposed color changes are already part of this PR 🙂

But having a closer look at the copy code button implementation, I would improve the following:

fballiano commented 4 months ago

I've tested your PR and it's needed so I'll merge it right away.

Personally I'd still keep the code in a "admin" related file but further improvements can be done for sure in a separate PR.