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

Holding click and moving disables selection in events sheet #1562

Closed
PascalLadalle opened this issue Mar 20, 2020 · 8 comments · Fixed by #1586
Closed

Holding click and moving disables selection in events sheet #1562

PascalLadalle opened this issue Mar 20, 2020 · 8 comments · Fixed by #1586
Labels
😤Non optimal UI A bug/issue where the UI is usable but not optimal

Comments

@PascalLadalle
Copy link

Describe the bug

When choosing an item in a list, if you release click on a different item than the one you pressed the click on, the selection doesn't occur.

Example:
image
Here, I press click on Ssmoke, move the cursor to Tsmoke and release the click.
Expected result: Tsmoke is selected.
Actual result: the menu closes but the object has not been selected.

Note: with the right-click actions/conditions menu, the item isn't selected either, but the menu remains open, which makes it less of a problem.

@4ian 4ian added the 😤Non optimal UI A bug/issue where the UI is usable but not optimal label Mar 21, 2020
@4ian
Copy link
Owner

4ian commented Mar 21, 2020

This is to be checked with the updated autocomplete that was merged in master.

@PawBud
Copy link
Contributor

PawBud commented Mar 21, 2020

I will verify it once.

@HarsimranVirk
Copy link
Contributor

HarsimranVirk commented Mar 23, 2020

@4ian this happens only with the inline editor, otherwise, autocomplete does not make a selection and the menu stays open. I have checked the demos, and this is how it is handled by the material-ui too.
Maybe this has something to do with the ClickAwayListener? I'd check and let you know :)
Yes, ClickAwayListener is the problem :) I removed it just to check and everything seems fine, also we can use touch to select options now, something which was not possible before. Anyway, I'll try to find some solution then :)

@4ian
Copy link
Owner

4ian commented Mar 24, 2020

Yes, ClickAwayListener is the problem :) I removed it just to check and everything seems fine, also we can use touch to select options now, something which was not possible before. Anyway, I'll try to find some solution then :)

Interesting! So the auto complete works well without ClickAwayListener, but when a ClickAwayListener is used (i.e: in InlinePopover), it's creating issues (can you recap which issue? on touch screen? Sorry I'm a bit confused in this thread)

As possible, we should have the autocomplete to work as the material-ui autocomplete.

@HarsimranVirk
Copy link
Contributor

it's creating issues (can you recap which issue? on touch screen? Sorry I'm a bit confused in this thread)

Alright @4ian, I'd try to explain what I could understand so far. Basically, ClickAwayListerner does not consider the menu of Autocomplete (or Downshift, too) as a part of the 'element'. So, when we click on the menu item, in the way described in this issue by @PascalLadalle, it is considered as a click outside of the element by the ClickAwayListener. Also, this is worse for touch, every touch on the menu is considered as a touch outside the element, so it's not possible to select options by touch in any way at all.
I think this is an issue which is more inclined towards material-ui rather than the implementation of it in GDevelop.

@4ian
Copy link
Owner

4ian commented Mar 24, 2020

I think this is an issue which is more inclined towards material-ui rather than the implementation of it in GDevelop.

I think so! Might be worth looking in material-ui issues if there is something that was reported.
Also, might be related to the fact that auto-complete is using a portal:

ClickAwayListerner does not consider the menu of Autocomplete (or Downshift, too) as a part of the 'element'.

Sounds like a portal issue.
There is a "disablePortal" prop in https://material-ui.com/api/autocomplete/ but I'm afraid this would make the list to show not properly.

But I think this is indeed more related to material-ui than GDevelop... could be good to report or see if we can have a workaround.

@HarsimranVirk
Copy link
Contributor

Sounds like a portal issue.

True. (I didn't know about portal before :p I looked through react and material-ui docs just now)

There is a "disablePortal" prop in https://material-ui.com/api/autocomplete/ but I'm afraid this would make the list to show not properly.

It does solve the issue but also makes a mess of the entire autocomplete component, the menu renders on top of the text field and there are scroll bars everywhere.

Screenshot from 2020-03-25 02-05-59

@4ian
Copy link
Owner

4ian commented Mar 24, 2020

It does solve the issue but also makes a mess of the entire autocomplete component, the menu renders on top of the text field and there are scroll bars everywhere.

Yep, not a convincing solution. I would rather see a solution being done in material-ui, seems like it's reported here: mui/material-ui#18586

Seems like the solution is complex/involves fixes/changes in React itself.
We might have to work-around this by for example trying to cancel the event propagation in autocomplete (can you try calling stopPropagation and preventDefault on the mouse/touch events).
As a last resort, use a setTimeout in InlinePopover to give a chance to children component to react to an event before being unmounted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
😤Non optimal UI A bug/issue where the UI is usable but not optimal
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants