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

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

Merged
merged 18 commits into from
May 1, 2024

Conversation

hirale
Copy link
Contributor

@hirale hirale commented Apr 27, 2024

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 In backend, add "click to copy text" feature #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

@github-actions github-actions bot added Template : admin Relates to admin template Component: Sales Relates to Mage_Sales Component: Adminhtml Relates to Mage_Adminhtml JavaScript Relates to js/* labels Apr 27, 2024
@pquerner
Copy link
Contributor

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
Copy link
Member

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
Copy link
Collaborator

kiatng commented Apr 28, 2024

+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
Copy link
Member

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

@github-actions github-actions bot added the Component: Page Relates to Mage_Page label Apr 28, 2024
@hirale
Copy link
Contributor Author

hirale commented Apr 28, 2024

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
Copy link
Contributor

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
Copy link
Contributor Author

hirale commented Apr 29, 2024

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

@fballiano
Copy link
Contributor

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
Copy link
Contributor Author

hirale commented Apr 29, 2024

@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
Copy link
Contributor

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

@pquerner
Copy link
Contributor

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
Copy link
Contributor

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

js/varien/js.js Outdated Show resolved Hide resolved
js/varien/js.js Outdated Show resolved Hide resolved
js/varien/js.js Outdated Show resolved Hide resolved
@fballiano
Copy link
Contributor

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

@hirale
Copy link
Contributor Author

hirale commented Apr 29, 2024

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

which browser?

@fballiano
Copy link
Contributor

chrome

@hirale
Copy link
Contributor Author

hirale commented Apr 29, 2024

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

@hirale
Copy link
Contributor Author

hirale commented Apr 29, 2024

@empiricompany
Copy link
Contributor

empiricompany commented Apr 30, 2024

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

js/varien/js.js Outdated Show resolved Hide resolved
@fballiano
Copy link
Contributor

In some resolutions it looks like this:

Screenshot 2024-04-30 alle 13 11 49

@fballiano
Copy link
Contributor

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

@hirale
Copy link
Contributor Author

hirale commented Apr 30, 2024

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

@fballiano fballiano changed the title Add copyText function to admin order view page for copying order details New feature: Added copyText function to admin order view page for copying order details Apr 30, 2024
fballiano
fballiano previously approved these changes Apr 30, 2024
Copy link
Contributor

@fballiano fballiano left a comment

Choose a reason for hiding this comment

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

latest version is fine to me :-)

fballiano
fballiano previously approved these changes Apr 30, 2024
@fballiano
Copy link
Contributor

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

@fballiano fballiano merged commit 25fd6bb into OpenMage:main May 1, 2024
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Adminhtml Relates to Mage_Adminhtml Component: Page Relates to Mage_Page Component: Sales Relates to Mage_Sales JavaScript Relates to js/* Template : admin Relates to admin template
Projects
None yet
Development

Successfully merging this pull request may close these issues.

In backend, add "click to copy text" feature
6 participants