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
863 stars 438 forks source link

New feature: Added copyText function to admin order view page for copying order details #3960

Closed hirale closed 2 months ago

hirale commented 2 months ago

Add copyText function to admin order view page for copying order details

Description (*)

Include the implementation of the feature described in issue #3952, allowing users to easily copy order numbers, customer emails, and tracking numbers directly from the order page with a single click.

Related Pull Requests

Fixed Issues (if relevant)

  1. Fixes OpenMage/magento-lts#3952

Manual testing scenarios (*)

  1. fresh installation with sample data
  2. add js function and modify templates
  3. tested on chrome, works fine

image

Questions or comments

pquerner commented 2 months ago

I would like this to be in a seperate template, so it can get used more easily (without duplicating essentially) across the backend. What do you think?

Flyingmana commented 2 months ago

can we do this over a special html attribute and css class combination, so the button is injected dynamically via js?

(just an idea, not a requirement from my side)

kiatng commented 2 months ago

+1 on @Flyingmana 's idea. Instead of svg, may be use the copy cursor on hover: image

See https://stackoverflow.com/a/62728944/488609

Flyingmana commented 2 months ago

The Icon used is already good, what I see nowdays it already evolves to a standard recognizable copy icon.

hirale commented 2 months ago

To add a copy icon to an element, follow these steps:

Add the data-copy-text attribute, which holds the text content to be copied.

For example:

<h4 class="icon-head head-account" data-copy-text="<?php echo $_order->getRealOrderId() ?>"><?php echo Mage::helper('sales')->__('Order # %s', $this->escapeHtml($_order->getRealOrderId())) ?> (<?php echo $_email ?>)</h4>
fballiano commented 2 months ago

I really like the idea of this PR, what about having the SVG in a CSS and then just inject a div/span placeholder that gets styled by the css?

hirale commented 2 months ago

I think it is more convenient to use the special attributes of html tag.

fballiano commented 2 months ago

ok i mean, instead of injecting the whole svg you're injecting the placeholder that gets stylized by the css, the data-attribute stay the same

hirale commented 2 months ago

@fballiano do you mean this?

js/varien/js.js

/**
 * Adds copy icons to elements with the class 'copy-text'.
 */
function addCopyIcons() {
    const copyTexts = document.querySelectorAll('.copy-text');
    copyTexts.forEach(copyText => {
        const iconStyle = JSON.parse(copyText.getAttribute('data-copy-icon'));
        const svg = createSVGElement(iconStyle);
        copyText.parentNode.appendChild(svg);
    });
}

/**
 * Creates an SVG element with the specified iconStyle.
 *
 * @param {Object} iconStyles - An object containing the style properties for the SVG icon.
 * @param {string} [iconStyles.cursor='pointer'] - The cursor style for the SVG element.
 * @param {string} iconStyles.height - The height of the SVG element.
 * @param {string} iconStyles.width - The width of the SVG element.
 * @param {string} [iconStyles.margin='0'] - The margin of the SVG element.
 * @return {HTMLElement} The created SVG element.
 */
function createSVGElement(iconStyles) {
    const copyIcon = document.createElement('span');
    copyIcon.classList.add('icon-copy');
    copyIcon.setAttribute('onclick', 'copyText(event)');
    copyIcon.style.cursor = iconStyles.cursor || 'pointer';
    copyIcon.style.height = iconStyles.height;
    copyIcon.style.width = iconStyles.width;
        copyIcon.style.margin = iconStyles.margin || '0';

    return copyIcon;
}
/**
 * Copies the text from the data-text attribute of the clicked element to the clipboard.
 *
 * @param {Event} event - The event object triggered by the click event.
 * @return {Promise<void>} A promise that resolves when the text is successfully copied to the clipboard.
 */
function copyText(event) {
    let copyText = event.currentTarget.previousElementSibling.getAttribute('data-text');
    navigator.clipboard.writeText(copyText);
}

phtml example

<h4 class="icon-head head-account copy-text" data-text="<?= $_order->getRealOrderId() ?>" data-copy-icon='{"cursor":"pointer","margin":"8px 0 0 5px","height":"16px","width":"16px"}'><?php echo Mage::helper('sales')->__('Order # %s', $this->escapeHtml($_order->getRealOrderId())) ?> (<?php echo $_email ?>)</h4>

skin/adminhtml/default/openmage/scss/override.scss

.icon-copy {
  display: inline-block;
  background-image: url('images/icon-copy.svg');
  background-repeat: no-repeat;
  background-size: contain;
}
fballiano commented 2 months ago

yep and the svg could also be inlined in the css (I kinda go for this route lately)

pquerner commented 2 months ago

yep and the svg could also be inlined in the css (I kinda go for this route lately)

So you love it when a base64 svg stares you in the face :D

fballiano commented 2 months ago

svg sprites are hard(er than old-style sprites) and if we start having many svg that will be a problem, if the svg is inlined in the css that's more or less solved

fballiano commented 2 months ago

when I click the copy icon on the order number I get:

Screenshot 2024-04-29 alle 22 46 47
hirale commented 2 months ago

when I click the copy icon on the order number I get: Screenshot 2024-04-29 alle 22 46 47

which browser?

fballiano commented 2 months ago

chrome

hirale commented 2 months ago

on chrome Version 124.0.6367.61 and firefox 125.0.2 (64-bit), works fine 2024-04-29 23-52-06

hirale commented 2 months ago

chrome

maybe here is the reason. Should I add some checks before writeText function? https://stackoverflow.com/questions/51805395/navigator-clipboard-is-undefined https://developer.mozilla.org/en-US/docs/Web/API/Clipboard/writeText https://developer.mozilla.org/en-US/docs/Web/Security/Secure_Contexts

fballiano commented 2 months ago

ahh maybe since in my local machine I don't have https, well I'd add a check as a first line in addCopyIcons

empiricompany commented 2 months ago

I can't easly fork your master... But i would propose to provide a visual feedback after copying. We can add a new class and then override the SVG icon like this

class="icon-copy icon-copy-copied"

demo

function copyText(event) {
    const copyIcon = event.currentTarget;
    const copyText = copyIcon.previousElementSibling.getAttribute('data-text');
    navigator.clipboard.writeText(copyText);
    copyIcon.classList.add('icon-copy-copied');
    setTimeout(() => {
        copyIcon.classList.remove('icon-copy-copied');
    }, 1000);
}
.icon-copy-copied {
  background-image: url('data:image/svg+xml,<svg xmlns="http://www.w3.org/2000/svg" x="0px" y="0px" width="100" height="100" viewBox="0 0 30 30"><path d="M 26.980469 5.9902344 A 1.0001 1.0001 0 0 0 26.292969 6.2929688 L 11 21.585938 L 4.7070312 15.292969 A 1.0001 1.0001 0 1 0 3.2929688 16.707031 L 10.292969 23.707031 A 1.0001 1.0001 0 0 0 11.707031 23.707031 L 27.707031 7.7070312 A 1.0001 1.0001 0 0 0 26.980469 5.9902344 z"></path></svg>');}
fballiano commented 2 months ago

nice idea! and then after a couple of seconds it could revert back to the original class

empiricompany commented 2 months ago

nice idea! and then after a couple of seconds it could revert back to the original class

i've update my code in the original comment to disapper after 1s

fballiano commented 2 months ago

In some resolutions it looks like this:

Screenshot 2024-04-30 alle 13 11 49
fballiano commented 2 months ago

@empiricompany I've committed your code to switch the icon

hirale commented 2 months ago

I've added the data-copy-text attribute to the target element and styled it using a CSS class, as we discussed earlier.

fballiano commented 2 months ago

@empiricompany I'd like to get your opinion on this last version :-)