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

Resolves #3849 #3850

Closed
wants to merge 4 commits into from
Closed

Resolves #3849 #3850

wants to merge 4 commits into from

Conversation

divyanash911
Copy link
Contributor

The moved variable seemed ambiguous. The erratic condition fell out of the if-else block since moved variable was creating a problem to ensure blocks collapsing.

@walterbender
Copy link
Member

Did you test when trying to drag a block from the collapse button? What action takes precedent? collapse or drag or both (or neither)?

@divyanash911
Copy link
Contributor Author

Did you test when trying to drag a block from the collapse button? What action takes precedent? collapse or drag or both (or neither)?

I looked at it.If I drag the block anywhere except the collapse button it moves properly. But if I use the collapse button to move blocks(I click in the area of collapse button to drag blocks) , it collapses as well. Also, the initial issue of erratic functioning of collapse button is gone. So, ideally if I click on the collapse button and use it to drag around the block should it collapse and drag it? Because a click was also recorded

@walterbender
Copy link
Member

Ideally, clicking on a block to move it would take precedence over collapse/expand. @pikurasa what do you think?

@divyanash911
Copy link
Contributor Author

Ideally, clicking on a block to move it would take precedence over collapse/expand. @pikurasa what do you think?

The issue is that the "move" detecting eventlistener "pressmove" is extremely sensitive and sometimes normal clicks are also recorded as "move" actions.

@divyanash911
Copy link
Contributor Author

@walterbender @pikurasa any updates on this?

@walterbender
Copy link
Member

Was waiting to hear back from @pikurasa

@pikurasa
Copy link
Collaborator

In terms of dragging from the collapse area of the block, I agree with @walterbender that dragging should take precedent. So, if you drag from that area, it should just drag; not drag and expand/collapse.

As for:

The issue is that the "move" detecting eventlistener "pressmove" is extremely sensitive and sometimes normal clicks are also recorded as "move" actions.

You may have a point here, because I often see kids "shake" the blocks when they are just trying to click on the block to trigger an event. I don't know exactly what the "sweet spot" would be for this. It probably differs between touch devices vs. using a mouse.

Copy link
Contributor Author

@divyanash911 divyanash911 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pikurasa have a look. For the "pressmove" event listener in block.js I have calculated displacement of the element and if it's less than a certain threshold(2) , I have ignored it as a drag action and considered it as a collapse action(using the "moved" variable)

@walterbender
Copy link
Member

@pikurasa would be great to see how some of the kids at MAP do with this setting. They'd be able to "tell" us if the threshold needs adjustment.

@divyanash911
Copy link
Contributor Author

@walterbender @pikurasa What is the future scope of this PR? Is there any plan on finding out an optimum threshold as suggested by @walterbender?

@walterbender
Copy link
Member

Devin will try to do some testing.

@pikurasa
Copy link
Collaborator

@divyanash911 I can test with some kids. This week is an off week for public schools in the US, so I may only be able to do tests with my (six-year-old) son.

I guess the one question I have is: Have you considered both laptop and mobile for this PR? Or is it just mobile?

I ask because if it's just mobile, I'll focus my testing there. If both are taken into account, I can test both.

@divyanash911
Copy link
Contributor Author

@divyanash911 I can test with some kids. This week is an off week for public schools in the US, so I may only be able to do tests with my (six-year-old) son.

I guess the one question I have is: Have you considered both laptop and mobile for this PR? Or is it just mobile?

I ask because if it's just mobile, I'll focus my testing there. If both are taken into account, I can test both.

The code changes majorly work for laptop bugs only I believe since the changes are in an if condition supported for laptops. I am unsure about shaky touches in mobile but will look into that as well.

@divyanash911
Copy link
Contributor Author

@walterbender @pikurasa Any updates?

@divyanash911
Copy link
Contributor Author

I have been working on the mobile touch thing as well but I dont find a lot of shake....are there any inputs on that?

@pikurasa
Copy link
Collaborator

@walterbender @pikurasa Any updates?

I can test this week.

I thought my son could help test, but it seems he has gotten rather used to using a track pad (and wasn't using the screen much).

@divyanash911
Copy link
Contributor Author

@walterbender @pikurasa Any updates?

I can test this week.

I thought my son could help test, but it seems he has gotten rather used to using a track pad (and wasn't using the screen much).

Oh okay! Do let me know on your insights after testing!

@pikurasa
Copy link
Collaborator

Oh okay! Do let me know on your insights after testing!

Testing this particular test, especially with a student cohort, has been harder than I was hoping, but "I'm working on it."

In the meantime, I'll be trying some A/B tests myself and getting Kai involved a little more.

@divyanash911
Copy link
Contributor Author

Oh okay! Do let me know on your insights after testing!

Testing this particular test, especially with a student cohort, has been harder than I was hoping, but "I'm working on it."

In the meantime, I'll be trying some A/B tests myself and getting Kai involved a little more.

Oh okay. I am currently busy with my final exams so I will also be unable to respond for the next few days!

Copy link

This pull request has been open for more than 60 days without any activity. It will be closed in 3 days unless the stale label is removed or commented on.

@github-actions github-actions bot added the Stale label Jun 27, 2024
Copy link

Closed pull request due to inactivity for more than 63 days.

@github-actions github-actions bot closed this Jun 30, 2024
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 this pull request may close these issues.

3 participants