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

Dropdowns from flyout don't disappear when scrolling #302

Closed
drigz opened this issue Apr 27, 2016 · 10 comments · Fixed by #738
Closed

Dropdowns from flyout don't disappear when scrolling #302

drigz opened this issue Apr 27, 2016 · 10 comments · Fixed by #738
Labels
Milestone

Comments

@drigz
Copy link
Collaborator

drigz commented Apr 27, 2016

dropdown scroll

Scrolling via the scrollbar removes the dropdown, but scrolling by the block area doesn't.

PS. Is it intentional that you can open dropdowns / edit fields in the flyout? Our UX guys were surprised by this behaviour.

@tmickel
Copy link
Contributor

tmickel commented Apr 27, 2016

Hmmmm thanks for catching that. I was pretty sure I considered this case but guess not.

Yes, it's absolutely intentional that you can edit things in the flyout. And eventually run the blocks there, too. This is Scratch UX - but curious why you were surprised by it

@tmickel tmickel added this to the Backlog milestone Apr 29, 2016
@tmickel tmickel removed their assignment Apr 29, 2016
@drigz
Copy link
Collaborator Author

drigz commented May 3, 2016

EDIT: this comment is here by mistake, and belongs in #277.

I've looked further into this, and found out two things.

I've put together a patch that allows the blocks to be dragged by the fields. However, I needed to differentiate between clicks and drags on the fields, and the cleanest way to do this was to make the behaviour in closeable flyouts the same as in fixed flyouts:

  • You can also scroll closeable flyouts by dragging horizontally.
  • Clicking a block in a closeable flyout no longer drops it directly into the workspace, but instead does nothing.

@tmickel, would these changes be undesirable?

Alternatively, we could just add a check in touchend that makes sure the touch is over the element (related Stack Overflow question).

@tmickel
Copy link
Contributor

tmickel commented May 3, 2016

@drigz I think those are actually desirable changes, and we'd be happy to put them in!

@drigz
Copy link
Collaborator Author

drigz commented May 6, 2016

@tmickel, is there any chance of a quick fix for this this week? It came up again in a UX review just now.

If you don't have time let me know where I should look and hopefully I can get around to it...

@rachel-fenichel
Copy link
Collaborator

Related: If you tap to open the dropdown in the flyout, then tap on the block it came from (or any other block), it won't close the dropdown but typing numbers won't actually change any of the blocks.

@drigz
Copy link
Collaborator Author

drigz commented May 7, 2016

I think the fix here is to add DropDownDiv.hide(/WithoutAnimation) by the calls to hideChaff in flyout.js.

@tmickel, how do you decide whether to animate the hide or not?

Why is DropDownDiv.hide not part of hideChaff? If it's because you want to be able to decide whether to animate, would it be safer to add an extra parameter opt_noAnimate, as for WidgetDiv.hide?

@rachel-fenichel, I think that's also true in the workspace. In general, this occurs when hideChaff closes the editor but DropDownDiv.hide is not called.

drigz added a commit that referenced this issue May 9, 2016
Fixes #302. This hides both WidgetDiv and DropDownDiv without animation,
otherwise they can float over other blocks as they're animating out.
@drigz
Copy link
Collaborator Author

drigz commented May 9, 2016

FYI, my commit doesn't fix the bug @rachel-fenichel pointed out, because it breaks due to races between onMouseDown (which hides the dropdown) and onMouseUp (which calls showEditor and hides or shows the dropdown).

@tmickel
Copy link
Contributor

tmickel commented May 9, 2016

@drigz In general we tried to animate out whenever the underlying thing wasn't going to move. So, for example, when you scroll the workspace, don't animate (or you get the problem observed here with the flyout as it's animating out).

My first desire was to put this all in hideChaff. I forget exactly what the issue was with that, but I think it was that hideChaff is being called in many places and it was hard to control the behavior as we wanted. I'd like to revisit the decision to have it out of hideChaff in the long run.

@rachel-fenichel I'm not sure actually what you're talking about - is that only on touch?

@drigz drigz closed this as completed in #323 May 9, 2016
thisandagain pushed a commit that referenced this issue May 17, 2016
…dered

Move Blockly.Block rendered into Blockly.BlockSvg
@luxaritas
Copy link

It looks like this is still an issue, though with the scroll wheel:

floating

I'm tempted to say that when using the scroll wheel it should just track the original input.

@tmickel
Copy link
Contributor

tmickel commented Nov 11, 2016

@lfp6 Yep, thanks for raising that!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants