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

Trash hotspot wrong when trash is full. #2237

Closed
NeilFraser opened this issue Jan 28, 2019 · 4 comments
Closed

Trash hotspot wrong when trash is full. #2237

NeilFraser opened this issue Jan 28, 2019 · 4 comments

Comments

@NeilFraser
Copy link
Member

  1. Open this page:
    https://beta-dot-blockly-games.appspot.com/music
  2. Drag a block over the trash. Explore the top/bottom/left/right boundaries of the hotspot. They are correct.
  3. Drop the block into the trash.
  4. Open the trash flyout, and drag the block out.
  5. Drag this block over the trash. Explore the top/bottom/left/right boundaries of the hotspot. They are incorrect.

Affects both RTL and LTR.

@NeilFraser
Copy link
Member Author

@BeksOmega I think this is yours?

@BeksOmega
Copy link
Collaborator

So digging into it I found that this is caused by the workspace calling recordDeleteAreas() when the trashcan lid is opened (i.e. when you click and it opens the flyout). This causes the trashcan to return the incorrect getClientRect() value, which changes the position of the hotspot.

This recordDeleteAreas call is being triggered by this line of code inside flyout.reflowInternal_(), because sometimes when a flyout reflows (i.e. zooms) the trashcan has to move. The thing is that the trashcan moving is bad behavior anyway, because it results in chasing, and was already fixed for bottom-anchored toolboxes.

So to fix this issue I'd like to anchor the trashcan & zoom controls to the start in end-anchored toolbox layouts and then remove the targetWorkspace_.resize() call.

@rachel-fenichel
Copy link
Collaborator

I agree, the root behaviour is bad, and fixing it would be better than adding in more special cases to mitigate it. So go for it.

@NeilFraser
Copy link
Member Author

Fix confirmed. Thanks!

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

No branches or pull requests

3 participants