Skip to content
This repository has been archived by the owner on Jul 22, 2024. It is now read-only.

Support for shaped clipped event handler #2633

Closed
wants to merge 3 commits into from

Conversation

keianhzo
Copy link
Contributor

@keianhzo keianhzo commented Jan 14, 2020

Adds a delegate to handle sticky shape clipped events for UI buttons.

This is a continuation of the work done for the honeycomb buttons input clipping based on the background shape.

You can appreciate the clipping in the URL bar buttons and the back buttons in the panels as they extend from UIBUtton. We can easily extend this to the rest of the UI buttons using the same delegate so all the input is background shape clipped.

Related to: #2601

@keianhzo keianhzo self-assigned this Jan 14, 2020
@keianhzo keianhzo requested a review from MortimerGoro January 14, 2020 10:07
@bluemarvin bluemarvin added this to the #9 polish milestone Jan 14, 2020
@bluemarvin
Copy link
Contributor

@keianhzo Looks like this needs to be rebased.

@keianhzo keianhzo force-pushed the v9/clipped_sticky_events branch from 26e66c9 to 34dcdc2 Compare February 5, 2020 09:54
Copy link
Contributor

@MortimerGoro MortimerGoro left a comment

Choose a reason for hiding this comment

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

If you click on a button, move the pointer outside and release the pointer outside it triggers the click action. As a user I'd expect the click to be cancelled if it was released outside of the button area

@MortimerGoro
Copy link
Contributor

@keianhzo master has also some instances of this behaviour but it got worse with this PR. Example:

  • Click on the home button
  • Move the pointer to back button
  • Release the pointer in back button
  • The home action is triggered in this branch but not in master

@MortimerGoro
Copy link
Contributor

Ideally we should cancel all the clicks if they are released outside the area (unless the button is small and needs some extra margin to avoid failing touches)

@bluemarvin
Copy link
Contributor

Ideally we should cancel all the clicks if they are released outside the area (unless the button is small and needs some extra margin to avoid failing touches)

But isn't this how we wanted sticky buttons to work? When you pressed in a button it would capture focus until it was released?

@MortimerGoro
Copy link
Contributor

But isn't this how we wanted sticky buttons to work? When you pressed in a button it would capture focus until it was released?

The problem we had is that after pressing if the pointer was moved the touch was cancelled and the click missed. For me the correct behavior is capture the focus until it's released, but if released outside of the button shape it should cancel the click. You can try the behavior in Oculus Launcher tray menu, it works that way.

@bluemarvin
Copy link
Contributor

Okay, to be clear we still want the keyboard keys to work the same, where even if you release outside of the key, it is still pressed. But for other bigger buttons, we don't want the button pressed if the pointer is outside the button?

@keianhzo keianhzo force-pushed the v9/clipped_sticky_events branch from 34dcdc2 to ae12697 Compare February 17, 2020 11:15
@keianhzo
Copy link
Contributor Author

@MortimerGoro I've updated the PR and added support for cancelling the events when released outside the button bounds.

@bluemarvin bluemarvin modified the milestones: #9 polish, #10 features Feb 25, 2020
@MortimerGoro
Copy link
Contributor

@keianhzo this one needs to be rebased too, conflicts with the changes for Android 8: // Android >8 doesn't perform a click when long clicking in ImageViews even if long click is disabled

@keianhzo
Copy link
Contributor Author

keianhzo commented Apr 9, 2020

I don't think this is worth landing. There seems to be a few side effect and the benefit doesn't justify the risk of regressions.

@keianhzo keianhzo closed this Apr 9, 2020
@bluemarvin bluemarvin deleted the v9/clipped_sticky_events branch July 15, 2020 20:23
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants