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

core: Dispatch rollOut/rollOver events on focus change and Tab #15955

Merged
merged 11 commits into from
Apr 21, 2024

Conversation

kjarosh
Copy link
Member

@kjarosh kjarosh commented Apr 11, 2024

Related to #5443, #15851.

When focus changes, rollOut/rollOver events should be dispatched. This includes onRollOut/onRollOver handlers, but also button actions.

Before After

This PR includes some code refactors & fixes:

  1. focus-related methods are moved to InteractiveObject,
  2. FocusTracker now operates on InteractiveObjects,
  3. mouse_over_object & mouse_down_object from UpdateContext were a little broken and were fixed,
  4. mouse hover was being updated a little too aggressively and was overriding tab hover.

Two new tests were added:

  1. avm1/tab_ordering_events
  2. avm1/tab_ordering_events_mouse

And one was updated (avm1/selection_handlers) to cover more cases.

More detailed descriptions are present in commit messages.

@kjarosh kjarosh force-pushed the tab-ordering-events branch 5 times, most recently from 2d1074a to fc4819a Compare April 17, 2024 18:43
@evilpie evilpie added the waiting-on-review Waiting on review from a Ruffle team member label Apr 21, 2024
}

impl<'gc> MouseData<'gc> {
pub fn hovered_object(&self) -> Option<InteractiveObject<'gc>> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think all these accessors are a bit overly verbose for how simple the struct is; on Discord I proposed making it a fully public struct, or going back to separate fields in UpdateContext (and just making them references)

Copy link
Member Author

Choose a reason for hiding this comment

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

Made it a fully public struct, as otherwise the fields would be differentiated by their position in that huge tuple instead of their type in update_context_params.

@kjarosh kjarosh force-pushed the tab-ordering-events branch from fc4819a to 147732e Compare April 21, 2024 22:26
kjarosh added 11 commits April 22, 2024 00:33
Only interactive objects may be focusable, so keeping these methods
in all display objects makes matters more difficult, as they also
has to be implemented for non-interactive objects.

Moving these to InteractiveObject simplifies code in the long run.
Since is_focusable has been moved to InteractiveObject,
it may now return true by default, because non-interactive objects
do not implement this method anymore.
Using Avm1::run_stack_frame_for_method for calling focus handlers
simplifies code and prevents duplication.
UpdateContext.mouse_over_object and UpdateContext.mouse_down_object
were treated differently from other properties. Changes made to them
were required to be applied manually after using the context.
However, when reborrowing the context, all changes made to these
properties were dropped, as they were not applied after reborrow.
Applying them after reborrow would be difficult (RAII, concurrency
problems), that's why they were replaced with mutable references.

MouseData struct was introduced to make the code more readable and
adhere to the convention of UpdateContext to contain mutable references
to higher level objects.
This is a utility method which simplifies getting the current focus
as an EditText object, which is being done frequently throughout Ruffle.
Originally, FocusTracker operated on DisplayObjects. However, in both
AVM1 and AVM2, all objects that are focusable are InteractiveObjects.
Switching FocusTracker to operate on InteractiveObjects simplifies code
in the long run.
Since objects may be hovered using Tab, mouse hover update cannot happen
every time, because it would always reset the hover set by Tab.

This is why mouse hover is skipped when the following are true:
1. the mouse has not moved,
2. no mouse button has been pressed,
3. there was a hover,
4. the hovered object did not disappear.
When focus changes, rollOut/rollOver events should be dispatched.
This includes onRollOut/onRollOver handlers, but also button actions.
Extend this test to cover rollOut/rollOver events too.
This test verifies events produced by various interactive objects
while tabbing.
This test verifies how the behavior of tabbing is integrated with
the mouse with respect to rollOut/rollOver events.
@kjarosh kjarosh force-pushed the tab-ordering-events branch from 147732e to 649f7b3 Compare April 21, 2024 22:34
@adrian17 adrian17 merged commit 1f5d199 into ruffle-rs:master Apr 21, 2024
17 checks passed
@kjarosh kjarosh deleted the tab-ordering-events branch April 21, 2024 22:53
@Lord-McSweeney Lord-McSweeney removed the waiting-on-review Waiting on review from a Ruffle team member label Apr 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

4 participants