Skip to content

[A11y] Stop sending the brightness that the app reports back to the app - #10071

Draft
victorsanni wants to merge 1 commit into
flutter:masterfrom
victorsanni:a11y-brightness-echo
Draft

victorsanni wants to merge 1 commit into
flutter:masterfrom
victorsanni:a11y-brightness-echo

Conversation

@victorsanni

Copy link
Copy Markdown

Part of #9893. Independent of #10065, which touches the same files.

The brightness dropdown on the Accessibility page wrote every change of its state back to the app, including changes that came from the app:

  • Opening the page wrote back the brightness that the app reported, which is the platform brightness unless it is overridden.
  • A hot restart lost the override that the dropdown set. The page wrote back the reset that ServiceExtensionManager makes when the main isolate exits, so the manager did not restore the override. The page then listened to a state that the manager no longer updates.
  • With Unify the brightness override with view metrics overrides flutter#195843, a viewMetricsOverride call that removes the brightness override posts a brightnessOverride event, which the page wrote back as a new override.

Now the dropdown calls AccessibilityController.setBrightness, so only a selection reaches the app, and the controller looks up the brightness state again whenever the main isolate changes. AccessibilityController.brightness is read-only.

Behavior change: the dropdown changes when the app or ServiceExtensionManager reports the change, not when a value is picked. At a breakpoint, it changes once the app resumes.

Caveat: ServiceExtensionManager still keeps the brightness that the app reports as an enabled state, because the extension has more than two values, and sets it again when the extension is registered again. So while DevTools is connected, the app stays at the brightness it last reported and stops following the system theme, even if nobody chose one. This is left for a follow-up PR.

Controller tests cover that the brightness the app reports is not written back, and that a hot restart restores the selection and the page follows the new isolate. The devtools_test fakes can now replace the main isolate and reset the service extension states, as ServiceExtensionManager does. There is no release note, because the page is behind the accessibility experiment flag.

Pre-launch Checklist

General checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read the Flutter Style Guide recently, and have followed its advice.
  • I signed the CLA.
  • I updated/added relevant documentation (doc comments with ///).

Issues checklist

Tests checklist

  • I added new tests to check the change I am making...
  • OR there is a reason for not adding tests, which I explained in the PR description.

AI-tooling checklist

  • I did not use any AI tooling in creating this PR.
  • OR I did use AI tooling, and...
    • I read the AI contributions guidelines and agree to follow them.
    • I reviewed all AI-generated code before opening this PR.
    • I understand and am able to discuss the code in this PR.
    • I have verifed the accuracy of any AI-generated text included in the PR description.
    • I commit to verifying the accuracy of any AI-generated code or text that I upload in response to review comments.

Feature-change checklist

  • This PR does not change the DevTools UI or behavior and...
    • I added the release-notes-not-required label or left a comment requesting the label be added.
  • OR this PR does change the DevTools UI or behavior and...
    • I added an entry to packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md.
    • I included before/after screenshots and/or a GIF demo of the new UI to my PR description.
    • I ran the DevTools app locally to manually verify my changes.

build.yaml badge

If you need help, consider asking for help on Discord.

The brightness dropdown wrote every change of its state back to the app,
including the brightness that the app reported and the reset that
ServiceExtensionManager makes when the main isolate exits. The dropdown
now calls AccessibilityController.setBrightness, and the controller looks
up the brightness state again whenever the main isolate changes.

This branch has not been deployed

No deployments
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

Successfully merging this pull request may close these issues.

1 participant