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

[web] Expose browser_detection through ui_web. #52380

Merged
merged 5 commits into from May 2, 2024

Conversation

ditman
Copy link
Member

@ditman ditman commented Apr 25, 2024

This PR moves the core of browser_detection.dart to a location in dart:ui_web so it can be used by apps and plugins.

In order for the code to be a little bit tidier in ui_web, it's encapsulated in a singleton instance that can be accessed through BrowserDetection.instance or a top level global browser in dart:ui_web.

Issues

Tests

Updated affected tests. Mostly the update was to call the methods from web_ui.browser.methodName rather than a global scope. Also split the tests for this module in two files:

  • engine_browser_detect_test.dart - with the tests specific to the engine (capability detection, etc...)
  • browser_detect_test.dart - only the tests pertaining to the "core" of the library.

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 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.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@ditman ditman requested a review from mdebbar April 25, 2024 00:20
@github-actions github-actions bot added the platform-web Code specifically for the web engine label Apr 25, 2024
@ditman ditman marked this pull request as ready for review April 25, 2024 01:31
lib/web_ui/lib/src/engine/browser_detection.dart Outdated Show resolved Hide resolved
lib/web_ui/lib/src/engine/browser_detection.dart Outdated Show resolved Hide resolved
lib/web_ui/lib/src/engine/browser_detection.dart Outdated Show resolved Hide resolved
lib/web_ui/lib/src/engine/browser_detection.dart Outdated Show resolved Hide resolved
ditman and others added 2 commits May 1, 2024 17:25
Co-authored-by: Mouad Debbar <mouad.debbar@gmail.com>
Updates documentation according to review comments.
@ditman ditman added the autosubmit Merge PR when tree becomes green via auto submit App label May 2, 2024
@auto-submit auto-submit bot merged commit b25f5d5 into flutter:main May 2, 2024
29 checks passed
@ditman ditman deleted the expose-browser-detection branch May 2, 2024 01:46
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 2, 2024
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request May 2, 2024
…147693)

flutter/engine@3087ec1...78dced5

2024-05-02 bdero@google.com et phone home (flutter/engine#52506)
2024-05-02 ditman@gmail.com [web] Expose browser_detection through ui_web. (flutter/engine#52380)
2024-05-02 jonahwilliams@google.com [Impeller] remove incremental allocation during filled path tessellation. (flutter/engine#52401)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC aaclarke@google.com,rmistry@google.com,zra@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request May 7, 2024
> [!IMPORTANT]
> Requires the following engine PR:
> * flutter/engine#52380
> ----

This PR refactors Flutter `foundation`'s library `platform` for the web with the same code we use to detect platforms in the engine.

## Issues

* Fixes: #128943

## Testing

Demo app deployed here:

* https://dit-browser-detect.web.app
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
autosubmit Merge PR when tree becomes green via auto submit App platform-web Code specifically for the web engine
Projects
None yet
2 participants