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

Fix network copy bug #5476

Merged
merged 5 commits into from
Mar 24, 2023
Merged

Fix network copy bug #5476

merged 5 commits into from
Mar 24, 2023

Conversation

CoderDake
Copy link
Contributor

Fixes #5192

I noticed while testing the network response and requests tabs that one could click copy while the data was still loading.
this could result in empty copies.

Another issue I seemed to intermittently see was that the data would still be empty even after data was loaded. This only happened intermittently thought.

This PR changes the copy buttons so that they won't show until there is data for them to copy, and also makes sure that they are more tightly coupled with the state of data being fetched.

@CoderDake CoderDake marked this pull request as ready for review March 22, 2023 16:10
@CoderDake CoderDake requested review from kenzieschmoll, bkonyi and a team as code owners March 22, 2023 16:10
Daniel Chevalier and others added 2 commits March 22, 2023 13:27
@CoderDake CoderDake merged commit 886a834 into flutter:master Mar 24, 2023
@@ -170,6 +170,37 @@ class HttpRequestView extends StatelessWidget {
}
}

/// A button for copying [DartIOHttpRequestData] contents.
///
/// If there is no content to copy, the button will not show. The copy contents
Copy link
Member

Choose a reason for hiding this comment

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

instead of hiding the button, we should just disable the onPressed handler. This will keep the UI consistent and not confuse users why sometimes the button is there and sometimes it is not. This is pretty standard across the rest of DevTools. For example, we don't remove a pause button when we are already paused, we just disable the onPressed handler so it shows up as disabled.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

SGTM
Issue created: #5519

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants