Problem
I’m wondering how to “beautify”/”simplify” this code:
function handleKeyDown (e) {
if (e.key === 'Enter') {
e.preventDefault()
myCustomEvent(e)
return
}
if (e.key === 'ArrowDown' || e.key === 'ArrowUp') {
e.preventDefault()
e.key === 'ArrowDown'
? document &&
document.activeElement &&
document.activeElement.nextElementSibling &&
document.activeElement.nextElementSibling.focus()
: document &&
document.activeElement &&
document.activeElement.previousElementSibling &&
document.activeElement.previousElementSibling.focus()
}
}
It seems too verbose to me. Is there something I’m doing wrong? How can I write it better?
Solution
Through clever usage of destructuring and using objects to store possible events, you can do this.
function focus(key){
const {activeElement:{[key]: elementSibling} = {}} = document;
if(elementSibling){
elementSibling.focus();
}
}
const ACTIONS = {
ArrowDown: () => focus('nextElementSibling'),
ArrowUp: () => focus('previousElementSibling'),
Enter: (e) => myCustomEvent(e)
}
function handleKeyDown (e) {
const handler = ACTIONS[e.key];
if(handler) {
e.preventDefault();
handler(e);
}
}
Here is a working example:
well I have a question, does document && document.activeElement
is mandatory?
If so, then there is a way to refactor it:
function handleKeyDown (e) {
if (e.key === 'Enter') {
e.preventDefault()
myCustomEvent(e)
return
}
if (e.key === 'ArrowDown' || e.key === 'ArrowUp') {
e.preventDefault()
focusSibling(e.key === 'ArrowDown' && document && document.activeElement)
}
}
function focusSibling(isNextElemSibling) {
if (isNextElementSibling && document.activeElement.nextElementSibling)
document.activeElement.nextElementSibling.focus()
else if (document.activeElement.previousElementSibling)
document.activeElement.previousElementSibling.focus()
}
Note: Probably you didn’t know, but when you do an if or a boolean (true or false) condition the program executes an AND &&
operator from left to right, and if the first value is false
, it stops the comparison, because if one value in an AND is false, the statement is false. So, that was a way to simplify your code.
If document && document.activeElement
is not mandatory, then only change this line:
focusSibling(e.key === 'ArrowDown')
I hope it helped you.
It may be wise to consider not omitting semicolons in your code
To streamline the code, you can try extracting the ‘focus’-function and use a small object to determine where to focus.
const tryToFocus = key => {
e.preventDefault();
const prevNext = {
ArrowDown: "nextElementSibling",
ArrowUp: "previousElementSibling"
};
if (Object.keys(prevNext).find(k => key === k)) {
document &&
document.activeElement &&
document.activeElement[prevNext[key]] &&
document.activeElement[prevNext[key]].focus();
}
}
function handleKeyDown (e) {
if (e.key === 'Enter') {
e.preventDefault(); // may be handled in myCustomEvent?
return myCustomEvent(e);
}
tryToFocus(e.key);
}