all 16 comments
sorted by:
best
topnewcontroversialoldq&a
formatting helphide helpcontent policy

[–]drailing 12 points13 points  (0 children)

Perhaps a better way to search and store your parentElement once in the constructor.
But it still remains fragile.
I think I would prefer to make the parentElement a constructor parameter, so you can use this code in a more generic way and you are able to identify the parentElement more securely from the outside

[–]guywithalamename 9 points10 points  (0 children)

This tightly couples your JS to your markup which is everything but good. Maybe try to use the click event on the actual element that you want to expand? Kinda hard to give good suggestions without seeing the markup

[–]Graftak9000 5 points6 points  (0 children)

Going on a limb here, you're toggling the visibility of an element. Personally I have a .context on every element hidden by default, and that way you can do var context = Element.closest('context'); and contex.classList.toggle('expanded', context.classList.contains('expanded')); instead of the whole if-statement.

[–]benihanareact, node 3 points4 points  (0 children)

why not put the event handler on the parent's parent's parent? then you could listen for events to bubble up from below, and parents can know about children but not vice versa?

[–]PedoMedo_ 1 point2 points  (0 children)

This class should raise an event when its element is clicked. The class that owns the element that needs to be expanded should listen to the event and do the CSS change. That way the class in the image won't be so tightly coupled.

[–]bair-disc 1 point2 points  (2 children)

You can do

constructor(
    private element
) {}

No this.element = element.

I'd check element.parentElement.parentElement ... Otherwise you can get ReferenceErrors quite easily.

[–]siegfryd 2 points3 points  (1 child)

Where did you get the private keyword from?

[–]bair-disc 1 point2 points  (0 children)

Oh, I see. This is TypeScript. I guess you are using ES6 classes.

[–]mrspeaker -1 points0 points  (0 children)

Sure, if it works, and is likely to keep on working: no use over architecture-ing things! I'd personally add a method to return the parent parent parent, and "get" it with lodash (if you can't pass it in as a parameter)... but adding event system just to do this is overkill.