current community

your communities

more stack exchange communities

company blog
Stack Exchange Inbox Reputation and Badges
tour help
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. Join them; it only takes a minute:

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top
up vote 3 down vote favorite

I have written some ES2015 JavaScript that adds a class to a root element when one of it's immediate children is focused.

Here is the markup for the code to be run on:

<div id="my-root-element">
    <input type="text">
    <button type="submit>
</div>

And the JS itself:

const rootElement = document.getElementById('my-root-element');

[...rootElement.children].forEach((element, _, children) => {
  const update = () => {
    const focused = children.indexOf(document.activeElement) !== -1;

    rootElement.classList[focused ? 'add' : 'remove']('focused');
  };

  [ 'focus', 'blur' ].forEach((eventName) => {
    element.addEventListener(eventName, () => setTimeout(update, 0));
  });
});

Are there any ways to "simplify" this code or generally make it smaller? I know it's short as it is currently but I understand that ES2015 has a lot of features that can help in writing short, concise code.

Here is an actual example of the code being used (focusing either the button or the text field should make the word "Focused!" appear):

const rootElement = document.getElementById('my-root-element');

[...rootElement.children].forEach((element, _, children) => {
  const update = () => {
    const focused = children.indexOf(document.activeElement) !== -1;

    rootElement.classList[focused ? 'add' : 'remove']('focused');
  };

  [ 'focus', 'blur' ].forEach((eventName) => {
    element.addEventListener(eventName, () => setTimeout(update, 0));
  });
});
.focused:after {
  content: 'Focused!';
}
<div id="my-root-element">
    <input type="text">
    <button type="submit">Submit</button>
</div>

share|improve this question
    
Is there a special reason why you used the underscore ( _ ) as name for the index in your forEach-loops? – mizech Jul 26 at 5:38
    
setTimeout with 0 ms? That calls it at once. What's the purpose of setTimeout? – mizech Jul 26 at 5:47
    
@mizech: I just used an underscore show it was unused, although I may be better naming it _index to prevent collisions. I used setTimeout(/* thing */, 0); as without it we end up having 1 frame where activeElement is none of the child elements when clicking between them. Clicking between 2 children without setTimeout: e1.onBlur -> focused is removed from root -> e2.onFocus -> focused is added to root. And with setTimeout: e1.onBlur -> setTimeout -> focused is left because activeElement is a child -> e2.onFocus – Jack Wilsdon Jul 26 at 15:22
    
Get the idea. Thanks for adding the explanations. – mizech 2 days ago
up vote 1 down vote

Well, for purpose of what you are trying to do I would say just CSS could do it.

Using the :focus selector and combining with a span element like this:

input:focus + span:after {
    content: 'Focused!';
}

/* just polishing the looks from here on */
button {
	display: block;
}
span {
  margin: 0 10px;
}
<div id="my-root-element">
    <input type="text"><span></span>
    <button type="submit">Submit</button>
</div>

About your JavaScript optimization question two things strikes me:

  • don't define functions inside loops if you don't need to.
  • I don't see why to use the setTimeout. Is there any browser mixing up the order of the events?

So a suggestion, without the function declaration inside the loop, using a closure and .toggle() instead of a ternary for add/remove:

const rootElement = document.getElementById('my-root-element');
((rootElement, children) => {
    const update = () => {
        rootElement.classList.toggle('focused', children.indexOf(document.activeElement) !== -1);
    };
    children.forEach(el => {
        ['focus', 'blur'].forEach((eventName) => {
            el.addEventListener(eventName, update);
        });
    });
})(rootElement, [...rootElement.children]);
share|improve this answer
    
The reason I'm using JS is because I want to change the style of both the button and the input when either the input or button is focused, which I can't do with CSS. I can style the button and input when the input is focused using the sibling operator (i.e. +) but I can't do it when the button is focused. – Jack Wilsdon Jul 26 at 13:32
1  
@JackWilsdon i see. Then I just suggest the JS changes above. I think your code looks quite nice from the beggining. – Sergio Jul 26 at 13:41
1  
I didn't know that toggle ( String [, force] ) had the force option, that's really cool thanks! – Jack Wilsdon Jul 26 at 15:25

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.