Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Match PlatformConfiguration properties to PlatformDispatcher ones #39685

Merged
merged 3 commits into from
Feb 23, 2023

Conversation

pdblasi-google
Copy link
Contributor

@pdblasi-google pdblasi-google commented Feb 16, 2023

  • Updated PlatformDispatcher to use PlatformConfiguration to back nativeSpellCheckServiceDefined and brieflyShowPassword
  • Updated web PlatformDispatcher/Configuration to match new API

Resolves #120840

Pre-launch 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 and followed the [Flutter Style Guide] and the [C++, Objective-C, Java style guides].
  • I listed at least one issue that this PR fixes in the description above.
  • I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See [testing the engine] for instructions on writing and running engine tests.
  • I updated/added relevant documentation (doc comments with ///).
  • I signed the [CLA].
  • All existing and new tests are passing.

@flutter-dashboard
Copy link

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

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

@flutter-dashboard flutter-dashboard bot added platform-web Code specifically for the web engine needs tests labels Feb 16, 2023
Copy link
Member

@goderbauer goderbauer left a comment

Choose a reason for hiding this comment

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

LGTM

@loic-sharma loic-sharma requested a review from ditman February 17, 2023 20:02
@loic-sharma
Copy link
Member

/cc @ditman for web_ui changes

Copy link
Member

@ditman ditman left a comment

Choose a reason for hiding this comment

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

The config code change, and exposing it through the platform dispatcher looks good, however the test for this looks a little bit weird to me? Isn't there a practical way of leveraging OOP more?

* Updated PlatformDispatcher to use PlatformConfiguration to back `nativeSpellCheckServiceDefined` and `brieflyShowPassword`
* Updated web PlatformDispatcher/Configuration to match new API
@Hixie
Copy link
Contributor

Hixie commented Feb 23, 2023

test-exempt: code refactor with no semantic change

@pdblasi-google pdblasi-google added the autosubmit Merge PR when tree becomes green via auto submit App label Feb 23, 2023
@auto-submit auto-submit bot merged commit cad5eec into flutter:main Feb 23, 2023
@pdblasi-google pdblasi-google deleted the issues/120840 branch February 23, 2023 21:42
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 23, 2023
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Feb 23, 2023
…121365)

* 2bc07cac7 Roll Skia from a321a8750271 to 1b2d815e9778 (5 revisions) (flutter/engine#39821)

* 57cbf0858 Roll Skia from 1b2d815e9778 to fff6c987d803 (2 revisions) (flutter/engine#39823)

* 2d2b51d69 Remove obsolete references in ViewConfiguration documentation (flutter/engine#39708)

* cad5eec1b Match PlatformConfiguration properties to PlatformDispatcher ones (flutter/engine#39685)
jason-simmons added a commit to jason-simmons/flutter_engine that referenced this pull request Feb 24, 2023
auto-submit bot pushed a commit that referenced this pull request Feb 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
autosubmit Merge PR when tree becomes green via auto submit App needs tests platform-web Code specifically for the web engine
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PlatformConfiguration does not contain brieflyShowPassword or nativeSpellCheckServiceDefined
5 participants