Style
I can not fault the style and layout.
The need to call init
which is an indirect call to handleClick
seams redundant. Do it in the constructor.
I am puzzled as to why you set readonly
to ''
(el.setAttribute('readonly', '');
) What has this to do with copying to the clipboard.
Names
Naming is very poor and also you use inconsistent naming conventions across JS (camelCase) and HTML (ids are all snake_case)
Names should provide the information needed to understand its behavior and content.
ActivationCodeComponent
is a bit obscure giving no indication of what it does.
.handleClick
Does not handle click (.copyToClipboard
does that) .handleClick
adds a click listener
el
Too short and obscure. Maybe inputEl
codeBox
What??? Maybe inputContainer
activationCodeContainer
is not containing anything. Maybe copyTextSrcEl
or textSourceElement
Basic rewrite
Rewrite using document.execCommand
Note that the rewrite does not return a use-able object thus no need to store it.
class ActivationCodeComponent{
constructor() {
function copyToClipboard () {
inputEl.value = textSrcEl.textContent;
inputContainer.appendChild(inputEl);
inputEl.select();
document.execCommand('copy');
inputContainer.removeChild(inputEl);
}
const inputEl = document.createElement('input');
const inputContainer = document.querySelector('#box');
const textSrcEl = document.querySelector('#activation_code');
document.querySelector('#copy_button').addEventListener("click", copyToClipboard);
}
}
new ActivationCodeComponent();
or
activationCodeComponent();
function activationCodeComponent() {
function copyToClipboard () {
inputEl.value = textSrcEl.textContent;
inputContainer.appendChild(inputEl);
inputEl.select();
document.execCommand('copy');
inputContainer.removeChild(inputEl);
}
const inputEl = document.createElement('input');
const inputContainer = document.querySelector('#box');
const textSrcEl = document.querySelector('#activation_code');
document.querySelector('#copy_button').addEventListener("click", copyToClipboard);
}
Problems
Deprecated
Document.execCommand is a depreciated function and should NOT be used.
Fix
You should be using Clipboard.writeText to add content to the clipboard. Note that the Code Review rewrite will not have clipboard permission and thus will fail.
Instantiatable and State
The best way to avoid bugs is to make impossible to incorrectly use the code.
Never allow conflicting or overlapping behaviors. As the code is, there are dozens of ways for it to be used in a manner that will create unpredictable behaviors.
Fix
There is no need to create an instantiatable object.
- Use a function's scope to contain the behavior and protect the state.
- Ensure that the function can only be called once.
User feedback
There is no user feedback to indicate if the button click has done anything. Always give feedback to all actions no matter how trivial.
Fix
The rewrite adds a simple message element that displays the result of the copy action. The message is cleared after 3 seconds.
Question
You write
"The validity of the script is too dependent on a set of HTML elements with the IDs I have used."
"Could it be made more flexible?"
There are many ways to make the code more flexible.
- Pass the code the elements required
- Use Element's Element.dataset to mark objects
Example marks the copy button with the query needed to locate the text source.
const copyBtn = document.querySelector("[data-copy-src]");
const copySrcEl = document.querySelector(copyBtn.dataset.copyButton);
<div class="code" id="activationCodeEl">B12A7</div>
<button id="copyButton" data-copy-src="#copySrcEl">Copy code</button>
Rewrite
Example moves element queries out of the function. To use you must pass all the elements required for the function to work.
The rewrite performs the following
- Marks the function to ensure that the code runs only once.
- Checks all elements to make sure that they are
instanceof Element
.
const isElement = obj => obj instanceof Element;
const query = (qStr, root = document) => root.querySelector(qStr);
const queries = (qObj, root) => (
Object.entries(qObj).forEach(([key, qStr]) => qObj[key] = query(qStr, root)),
qObj
)
const copyElements = queries({
button: "#copyButton",
textSrcEl: "#activationCodeEl",
messageEl: "#copyMessageEl",
});
copyToClipboard(copyElements);
function copyToClipboard({button, textSrcEl, messageEl}) {
const okMessage = "Text copied to system copy buffer";
const failMessage = "Text failed to copy";
const okTypeClass = "copyOK", failTypeClass = "copyFail";
var messageClearHdl;
if (!isElement(button) || !isElement(textSrcEl) || !isElement(messageEl)) { return }
if (copyToClipboard.active) { return }
copyToClipboard = Object.freeze(
Object.assign(copyToClipboard, { active: Symbol.for("activeCopy") })
);
function showMessage(text, typeClass) {
clearTimeout(messageClearHdl);
messageEl.textContent = text;
messageEl.className = typeClass;
messageClearHdl = setTimeout(() => {
messageEl.textContent = "";
messageEl.className = "hide";
}, 3000);
}
button.addEventListener("click", () =>
navigator.clipboard.writeText(textSrcEl.textContent).then(
() => showMessage(okMessage, okTypeClass),
() => showMessage(failMessage, failTypeClass)
));
}
.container {
text-align: center;
}
#box {
border: 1px solid #ccc;
width: 200px;
margin: 20px auto;
padding: 0 10px;
}
.code {
letter-spacing: 2px;
color: #069;
font-size: 16px;
font-weight: bold;
}
.hide {
display: none;
}
.copyOK {
margin-top: -18px;
font-family: arial;
display: block;
}
.copyFail {
margin-top: -18px;
color: red;
font-family: arial;
display: block;
}
<div class="container">
<div id="box">
<div class="label text-uppercase small">Your activation code</div>
<div class="code" id="activationCodeEl">B12A7</div>
</div>
<div id="copyMessageEl" class="hide"></div>
<button id="copyButton" class="btn btn-sm btn-success text-uppercase">Copy code</button>
</div>
Of course many will think that the context security is over the top. Apart from guaranteeing the code will behave as expected there is no need to include the context security.
const query = (qStr, root = document) => root.querySelector(qStr);
const queries = (qObj, root) => (
Object.entries(qObj).forEach(([key, qStr]) => qObj[key] = query(qStr, root)), qObj);
// example only so not making call
//copyToClipboard(queries({
//button: "#copyButton", textSrcEl: "#activationCodeEl", messageEl: "#copyMessageEl",
//}));
function copyToClipboard({button, textSrcEl, messageEl}) {
const okMessage = "Text copied to system copy buffer";
const failMessage = "Text failed to copy";
const okTypeClass = "copyOK", failTypeClass = "copyFail";
var messageClearHdl;
function showMessage(text, typeClass) {
clearTimeout(messageClearHdl);
messageEl.textContent = text;
messageEl.className = typeClass;
messageClearHdl = setTimeout(() => {
messageEl.textContent = "";
messageEl.className = "hide";
}, 3000
);
}
button.addEventListener("click", () => {
navigator.clipboard.writeText(textSrcEl.textContent).then(
() => showMessage(okMessage, okTypeClass),
() => showMessage(failMessage, failTypeClass)
);
});
}