prototype ignore semantics actions by chunhtai · Pull Request #37199 · flutter/flutter

chunhtai · 2019-07-29T22:30:39Z

Description

This is a prototype.

expects behavior as following

if ignoringSemantics = true
     no semantics segmantics should be collected from the subtree
else
     if ignoring= true
           collect all the semantics except tap and longpress
    else
           collect all semantics

Related Issues

#37162

Tests

I added the following tests:

Replace this with a list of the tests that you added as part of this PR. A change in behaviour with no test covering it
will likely get reverted accidentally sooner or later. PRs must include tests for all changed/updated/fixed behaviors. See [Test Coverage].

Design doc https://docs.google.com/document/d/1WuVBc58_wV6__ZCe1NgYsnvTJtSdOtVcwSzPwM_YT74/edit?usp=sharing

fluttergithubbot · 2019-07-29T22:30:44Z

It looks like this pull request may not have tests. Please make sure to add tests before merging. While there are exceptions to this rule, if this patch modifies code it is probably not an exception.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

/cc @dnfield

goderbauer

It may be worthwhile to write a little design document for this to reflect on what the goals for this API are and to answer some of the open questions.

goderbauer · 2019-07-30T09:12:28Z

Why only block tap and longPress? I think a RenderIgnorePointer also blocks scrolling and other gestures, no?

We will probably want to block more, This is more like a place holder for now

goderbauer · 2019-07-30T09:13:09Z

Do we have a use case were we only need to block selective actions? Or is it all-or-nothing?

If we go the down the selective route - how do we deal with newly added semantics actions that people didn't know about when writing their render objects that use this functionality?

I believe we might want to do selective route for example we are not likely to block
SemanticsAction.showOnScreen in ignoring pointer widget.

For newly added semantics actions, I really can't think of a comprehensive way to improve this. we can try to group the actions into different subgroup, for example, we can have a subgroup called PointerActions that will include all the pointer related semantics action, and the the renderobject can directly mute the subgroup instead individual actions. The person who add the new semantics action need to also include the action into the correct subgroup.

showOnScreen is kind of a special SemanticsAction since it is not directly user-triggered. It would probably be good to have a way to block all user-initiated actions.

Having said that, showOnScreen as a semantics action is really only relevant if the node is scrollable. And scrolling is one of the actions we are blocking, with ignorepointer, right?

goderbauer · 2019-07-30T09:15:49Z

I wonder if RenderIgnorePointer could set "blockedActions" on the SemanticsConfiguration it returns from describeSemanticsConfiguration instead of adding this API here. The semantics compiler would than have to resolve this as it rebuilds the tree based on that changed configuration...

Is this possible? We can do this if we calculate the subtree's blockedActions without any caching. If we want to use cache, we need a way to invalidate subtree's caches. I can't think of good way to do that if we didn't do it in renderobject level.

It may make more sense to cache these things on the SemanticsNode itself. We already do this for other things, like the elevation imposed by a parent on its children or the parent paint clip:
https://master-api.flutter.dev/flutter/semantics/SemanticsNode/elevationAdjustment.html
https://master-api.flutter.dev/flutter/semantics/SemanticsNode/parentPaintClipRect.html

It would probably be more idiomatic to have specify the blocked actions on the SemanticsConfiguration and cache things on the SemanticsNode.

those case are setting semantics configuration for compilechildren to finalize the value. However, the blocked action I did here is blocking during semantics configuration setup. The biggest difference is that make node dirty will re compile its children SemanticsFragment but it does keep the old cache of semantics configuration. in the case of elevationAdjustment or parentPainterClipRect, it is ok because the subtree semantics configuration did not change. it just need to recompile. However, in my case it will break because the subtree need to reconfig semantics configuration. I think if we don't want to recursively mark subtree dirty, we can try to strip out blocked semanticsactions during compilechildren. I am not sure if there is any side effect tho.

goderbauer · 2019-07-30T09:21:41Z

Just the parent? Or all ancestors?

All ancestors, sorry for the confusing

goderbauer · 2019-07-30T09:25:34Z

How is the cache invalidated if the parent's _inheritedMutedSemanticsActions change?

the recursive markRenderObjectNeedsSemanticsUpdate should invalidate all the subtree cache.

That is a departure from how SemanticsUpdate marking works right now. Right now, if node A is marked as needing an update, all its children will also be updated without requiring to be marked as needing an update.

The subtree could potentially be very large (think about a modal dialog that blocks interactions with everything in the route below it), so performance-wise it may not be a good idea to walk everything.

goderbauer · 2019-07-30T09:26:03Z

How is it guaranteed that the parent's _inheritedMutedSemanticsActions are up-to-date at this point?

if any ancestor has change, its subtree should have all their _cachedMutedSemanticsActions set to null

goderbauer · 2019-07-30T09:26:54Z

I believe you do not need to walk down to the leaves. If you mark a node as needing semantics update, the entire subtree gets re-build anyways starting with the marked node.

its subtree already cached both _cachedMutedSemanticsActions and _cachedSemanticsConfiguration which will need to be removed by calling markNeedsSemanticsUpdate. I feel a bit scared to manually remove the cache without going through markNeedsSemanticsUpdate.

justinmc · 2019-07-31T00:11:46Z

I think you could do .containsAll. Same thing either way though.

chunhtai · 2019-08-05T18:16:11Z

Refactored @goderbauer

chunhtai · 2019-08-19T21:24:05Z

readOnly in EditableText will make it not an ediablebox

chunhtai · 2019-08-20T22:29:15Z

@goderbauer finalize the prototype, and updated design doc. Test will be added after we decide this is the route we want to pursue.

chunhtai · 2020-03-03T00:12:42Z

closed for now until i have time to get back to it

googlebot added the cla: yes label Jul 29, 2019

fluttergithubbot added a: accessibility Accessibility, e.g. VoiceOver or TalkBack. (aka a11y) p: cupertino_ui cupertino_ui package in flutter/packages p: material_ui material_ui package in flutter/packages labels Jul 29, 2019

fluttergithubbot added the framework flutter/packages/flutter repository. See also f: labels. label Jul 29, 2019

chunhtai mentioned this pull request Jul 29, 2019

a11y: disabled textfield not announced as textfield #37162

Closed

goderbauer reviewed Jul 30, 2019

View reviewed changes

justinmc reviewed Jul 31, 2019

View reviewed changes

chunhtai force-pushed the issues/37162 branch 2 times, most recently from 75ee4f7 to 4903c0e Compare August 5, 2019 18:12

chunhtai commented Aug 19, 2019

View reviewed changes

chunhtai force-pushed the issues/37162 branch from 8248569 to 004ded6 Compare August 19, 2019 22:02

chunhtai added 2 commits August 20, 2019 09:51

prototype ignore semantics actions

8a225a4

refactor

dbce220

chunhtai force-pushed the issues/37162 branch from 2469c25 to dbce220 Compare August 20, 2019 17:09

remove accidently added file

b0ae887

chunhtai closed this Mar 3, 2020

chunhtai mentioned this pull request Jun 15, 2020

TextField semantics are gone when being disabled #59402

Closed

chunhtai mentioned this pull request Jul 24, 2020

prototype ignore semantics actions #62235

Closed

13 tasks

github-actions Bot locked as resolved and limited conversation to collaborators Aug 1, 2021

Conversation

chunhtai commented Jul 29, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issues

Tests

Uh oh!

fluttergithubbot commented Jul 29, 2019

Uh oh!

goderbauer left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chunhtai commented Aug 5, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chunhtai commented Aug 20, 2019

Uh oh!

chunhtai commented Mar 3, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

chunhtai commented Jul 29, 2019 •

edited

Loading