Skip to content

Detect Flutter Web apps and embed inspector page for them #1539

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

Merged
merged 4 commits into from
Mar 16, 2022

Conversation

elliette
Copy link
Contributor

This is still just restricted to internal apps at the moment.

@elliette elliette assigned annagrin and unassigned annagrin Mar 11, 2022
@elliette elliette requested a review from annagrin March 11, 2022 01:24
Copy link
Contributor

@annagrin annagrin left a comment

Choose a reason for hiding this comment

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

Thanks Elliott! Is there a way to add tests for the new panel? I think we have extension tests somewhere.

<script src="panel.js"></script>
</body>

</html>
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need a new line here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think so - it is automatically removed by the formatter when I save the file

@annagrin annagrin self-requested a review March 11, 2022 01:57
Copy link
Contributor

@annagrin annagrin left a comment

Choose a reason for hiding this comment

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

LGTM with a small comment

@elliette
Copy link
Contributor Author

Thanks Elliott! Is there a way to add tests for the new panel? I think we have extension tests somewhere.

I have an open TODO to see if that's possible: https://github.com/dart-lang/webdev/blob/master/dwds/test/debug_extension_test.dart#L130-L132 WebDriver doesn't support interacting with Chrome DevTools, so with our current test infrastructure there is no way to test for it.

Copy link
Contributor

@annagrin annagrin left a comment

Choose a reason for hiding this comment

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

LGTM with a comment.

// If Flutter is not detected after 10 seconds, still
// create the Dart Debugger panel:
clearInterval(checkFlutterAppInterval);
chrome.devtools.panels.create(
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like for flutter the debugger panel is not created until the inspector one is available. Does it make sense to show the debugger panel always, and add the inspector panel when we can?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed to that!

@elliette elliette merged commit 066f393 into dart-lang:master Mar 16, 2022
@elliette elliette deleted the detect-flutter-web branch May 4, 2022 22:38
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.

2 participants