-
Notifications
You must be signed in to change notification settings - Fork 361
Network: add a banner indicating when we are not logging network requests #9495
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
Conversation
elliette
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Request to use a Card instead of BannerInfo
| } | ||
| } | ||
|
|
||
| class _NetworkLoggingPausedMessage extends BannerInfo { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know if we are getting much benefit from extending BannerInfo, since it's really designed to be used with the banner controller (that's why we have to pass in parameters that then aren't being used, like the key and dismissOnConnectionChanges)
Instead, I think we could simply display a Card widget here with the "Traffic recording..." text and no X button.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done!
| if (!_recording) | ||
| Padding( | ||
| padding: const EdgeInsets.only(top: intermediateSpacing), | ||
| child: Card( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: pull Card into a separate StatelessWidget (e.g. RecordingPausedBanner) otherwise LGTM!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done!
|
|
||
| ## Network profiler updates | ||
|
|
||
| * Fixed layout of the "error count" badge in the tab name. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missed this for the other PRs - these should have a PR attached (see examples above in file). However, this can be addressed when we put together the release notes and not necessarily in this PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done!
Cherry-picked commits in DevTools repo include: * flutter/devtools@1b39a2e "Add basic support for Flutter web applications served with -d web-server" flutter/devtools#9468 * flutter/devtools@fb4fa1f "Handle shortcuts for actions like copy/paste when embedded inside VS Code" flutter/devtools#9472 * flutter/devtools@cd14e27 "Fix position and width of "error count" badge in Network in app bar" flutter/devtools#9470 * flutter/devtools@6b9ba48 "Add a horizontal scrollbar to DevTools tables" flutter/devtools#9482 * flutter/devtools@1f8b3f5 "Disable screens not compatible with DWDS websocket mode" flutter/devtools#9481 * flutter/devtools@8a42447 "Table columns are resizable" flutter/devtools#9485 * flutter/devtools@5c0b17a "Network panel: disable Header ExpansionTiles when no data" flutter/devtools#9492 * flutter/devtools@0dbead1 "Network: add a banner indicating when we are not logging network requests" flutter/devtools#9495 Issue description: Users were unable to use various functions in the DevTools Network screen, including copy+paste, view requests, and view the request table when embedded in a small window. What is the fix: Various fixes are included for these user issues. Why cherry-pick: Users reported these issues in the user survey pre-dating Flutter 3.35; it is important for user trust to fix these in Flutter 3.38. Risk: Low, this fix only affects DevTools. Issue link(s): Dart-Code/Dart-Code#3488, flutter/devtools#7253, flutter/devtools#9435, flutter/devtools#8920, flutter/devtools#9465, flutter/devtools#9488, flutter/devtools#9464 Cherry-pick: flutter/devtools@v2.51.0...2.51.1 Change-Id: Iae696151778849fdb6faf6bf83a123f518fee77c Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/458020 Reviewed-by: Devon Carew <devoncarew@google.com> Commit-Queue: Samuel Rawlins <srawlins@google.com> Reviewed-by: Elliott Brooks <elliottbrooks@google.com>
Work towards #9489. There's not much to say... it's displayed below the controls:
Definitely review this with ?w=1 in the URL.