day8 / re-com

A ClojureScript library of reusable components for Reagent
https://re-com.day8.com.au
MIT License
797 stars 147 forks source link

Typeahead suggestions container shifts everything below it #130

Closed p-himik closed 7 years ago

p-himik commented 7 years ago

If you go to http://re-demo.s3-website-ap-southeast-2.amazonaws.com/#/typeahead and via the Developer tools add some items below the typeahead main div, typing anything in the typeahead will open the suggestions container that in turn will shift the items you've added down. I guess position: absolute needed to be added, but I'm not that proficient in CSS to create a correct pull request.

Gregg8 commented 7 years ago

Yes, I can see what you mean and that you might not want the dropdown part to push everything down but to appear on top of what's underneath.

Because changing this could cause UI breakage, I would like to join the author of this component, @blak3mill3r into this conversation.

How would this change affect your UI?

I think it makes sense for the dropdown section to be position: absolute, just like our single-dropdown. Otherwise you need to leave a big empty space underneath for the list.

Or do you get around this another way?

blak3mill3r commented 7 years ago

@p-himik @Gregg8

Sorry if this is obvious... I can't see this behavior. Exactly which element on the demo page do you add more elements underneath? I tried adding it to the end of the v-box that contains the typeahead on the demo page, but I didn't see this behavior (instead it seems that the new element pushes the suggestions down so they're no longer flush with the text-input).

I'm tempted to say that position: absolute is the right idea, I'd like to try it out first and see if it breaks anything else.

Gregg8 commented 7 years ago

In the demo, I added the component:

[label :label "UNDERNEATH"]

Directly underneath the [typeahead] component (yep, last component in the containing [v-box]) and that demonstrates the behaviour:

Initial state:

image

Enter the text 'left':

image

The label component has been pushed down.

This is a trivial example, but if the typeahead was in a form of vertically laid out components, it would push all components underneath it down below the resultant list.

We are suggesting this list should appear "on top" and only take up as much vertical space as the height of the text-box and therefore not push things down, just like the way the [single-dropdown] works.

Gregg8 commented 7 years ago

On re-reading @p-himik's initial description, I may not be interpreting it exactly as he describes it but I maintain my suggestion that the [typeahead] would be improved it it worked like the [single-dropdown].

@p-himik ?

p-himik commented 7 years ago

That's exactly what I meant, thanks for the images and clarification.

nidu commented 7 years ago

I'd also like to note that clicking around dropdown list doesn't close the list. Don't know if this is intentional behaviour but after most dropdowns it's just an intuition.

blak3mill3r commented 7 years ago

@p-himik @Gregg8 I think that position: absolute; CSS is the right thing to do. I don't have time to work on this at the moment, but I will be happy to do so. Feel free to do it yourself of course.

When I wrote the typeahead, I remember wanting to make it close on a click event outside the component as @nidu described, but it wasn't obvious to me how to accomplish that with reagent. For the dropdown, I believe this behavior comes free with the built-in behavior of the <select> DOM element, but as the typeahead suggestions are plain old non-form elements with javascript it would have to be implemented explicitly. It just now occurs to me that the "right" way to do this is probably a blur handler on the <input> in the typeahead. I don't know why I didn't think of that before... I had been thinking of a click handler on document.body or something, which sounded gross.

@nidu Open another issue for that, if you like. I'll do it at some point.

nidu commented 7 years ago

@blak3mill3r Another general question that i suppose doesn't deserve dedicated issue: didn't you think to just add option to asynchronously load data to single-dropdown component because as for me it looks like the only advantage of typeahead component (really don't wanna offend or neglect work done with typeahead)? Moreover look and feel would be more coherent because now components looks different for no reason i suppose.

blak3mill3r commented 7 years ago

@nidu Thanks for pointing that out... that did not occur to me when writing the typeahead component. This might be because I lacked familiarity with single-dropdown (I still am not familiar with it).

A lot of UI widget libraries provide a typeahead component, I needed one in re-com, and wrote it more or less from scratch (although it does re-use the text-input component). If it is possible to merge the functionality into single-dropdown without bloating/confusing the code for that component, that would probably be a Good Thing.

Gregg8 commented 7 years ago

This has now been fixed. Closing.