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

x-topic-search and x-interaction challenges #239

Open
dan-searle opened this issue Jan 24, 2019 · 2 comments
Open

x-topic-search and x-interaction challenges #239

dan-searle opened this issue Jan 24, 2019 · 2 comments
Labels
help wanted Extra attention is needed question Further information is requested

Comments

@dan-searle
Copy link
Contributor

dan-searle commented Jan 24, 2019

In order for x-topic-search's results to be closed when you interact with something else on the page, we need do something like this:

['focusout', 'focusin', 'click'].forEach(action => {
	document.body.addEventListener(action, event => {
		if(!componentRootNode.contains(event.target)) {
			topicSearchActions.hideResult();
		}
	});
});

(We can't just listen for the blur of the topic search input, as this will cause the search results to close when you click inside the search results (e.g. on the follow button).)

So, there's a couple of questions:

  1. What is an acceptable way for an x-dash component to add (and remove) DOM event listeners outside of themselves? (we should avoid it if we can, but in this case it seems unavoidable).
  2. How to pass a ref to a DOM node from the wrapped component to the action? E.g. the event handler has this condition: componentRootNode.contains(event.target)
@dan-searle dan-searle added help wanted Extra attention is needed question Further information is requested component A new component in development labels Jan 24, 2019
@i-like-robots
Copy link
Contributor

i-like-robots commented Apr 10, 2019

So I'll speak for FT.com... I think it's fine to use regular class components, add these listeners when the component is mounted, and remove them when unmounted. I'm sure this is not new advice to you but I guess this is the happy path. #248 indicates this may also be the way forward.

I can't offer any insight as to whether this approach would or would not work in the app.

@apaleslimghost
Copy link
Member

that approach is fine for the app, but not exposing class components means we have much more control over statically verifying that components aren't doing bad things

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants