devtools icon indicating copy to clipboard operation
devtools copied to clipboard

inspector add screenshot

Open mengdouer opened this issue 3 years ago • 7 comments

This pr is a screenshot of the current widget that can be displayed on the inspect screen when you double-click its widget. Below is its demo. Inside our company, this feature is very helpful in debugging. It's still in the draft stage, not sure how you feel about this change, any thoughts on this can be discussed here.

https://user-images.githubusercontent.com/58758250/180193329-99c3a138-051e-45fe-ba39-386dda4e4ee7.mov

Pre-launch Checklist

  • [x] I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • [x] I read the Tree Hygiene wiki page, which explains my responsibilities.
  • [x] I read the Flutter Style Guide recently, and have followed its advice.
  • [x] I signed the CLA.
  • [ ] I listed at least one issue that this PR fixes in the description above.
  • [ ] I updated/added relevant documentation (doc comments with ///).
  • [ ] I added new tests to check the change I am making, or this PR is test-exempt.
  • [ ] All existing and new tests are passing.

If you need help, consider asking

for advice on the #hackers-new channel on Discord.

mengdouer avatar Jul 21 '22 10:07 mengdouer

Thanks for the PR!

@jacob314 @InMatrix - is this something we've considered adding to the inspector in the past? Any thoughts on this UX?

kenzieschmoll avatar Jul 21 '22 14:07 kenzieschmoll

@jacob314 @InMatrix - is this something we've considered adding to the inspector in the past? Any thoughts on this UX?

We've considered this kind of widget screenshot in the HotUI experiment: https://docs.google.com/document/d/1ZaHqKnON8-WEhke3Y6FpHeuB5BNlxDQj1cCYB1SoI_g/edit#heading=h.pub7jnop54q0

About the specific UI, I wonder if we can solve the same problem by hooking up widget selection in the tree view to the on-device widget highlighter. Essentially, when you select a node in the tree, the corresponding widget will get highlighted on the device/simulator automatically, allowing you to identify the widget. @zzm990321 Is this what you use the widget screenshots for? It would be helpful if you can elaborate on your use case a bit more.

InMatrix avatar Jul 26 '22 22:07 InMatrix

I'd added that API to Flutter for use cases similar to this but never had time to implement in DevTools. Some things to consider:

  1. Should polish to handle widgets with transparent backgrounds better. For example, see what it does when selecting a text widget.
  2. Consider showing the screenshot without a click when viewing the layout explorer view.
  3. Consider alternate UX of showing a screenshot of the full app in the inspector and highlighting the bounding box for the selected widget.

jacob314 avatar Jul 26 '22 22:07 jacob314

@jacob314 @InMatrix - is this something we've considered adding to the inspector in the past? Any thoughts on this UX?

We've considered this kind of widget screenshot in the HotUI experiment: https://docs.google.com/document/d/1ZaHqKnON8-WEhke3Y6FpHeuB5BNlxDQj1cCYB1SoI_g/edit#heading=h.pub7jnop54q0

About the specific UI, I wonder if we can solve the same problem by hooking up widget selection in the tree view to the on-device widget highlighter. Essentially, when you select a node in the tree, the corresponding widget will get highlighted on the device/simulator automatically, allowing you to identify the widget. @zzm990321 Is this what you use the widget screenshots for? It would be helpful if you can elaborate on your use case a bit more.

Thank you for your reply. I think it's good to display it on the device too, but viewing it on devtools makes it clear that only a widget is viewed,it will not be affected by other displays and this image can be saved.

mengdouer avatar Jul 28 '22 02:07 mengdouer

I'd added that API to Flutter for use cases similar to this but never had time to implement in DevTools. Some things to consider:

  1. Should polish to handle widgets with transparent backgrounds better. For example, see what it does when selecting a text widget.
  2. Consider showing the screenshot without a click when viewing the layout explorer view.
  3. Consider alternate UX of showing a screenshot of the full app in the inspector and highlighting the bounding box for the selected widget.

Sincerely thank you for your advice.

  1. Yes, the transparent component display is not good, I will optimize it later.
  2. Can we add a tab to the right of the Details Trees so that when the component is selected we can view the screenshot without additional clicks. image
  3. Explicitly wanting to see a screenshot of a single component I think is sometimes useful too. I was also able to display both images to devtools. I think it would still be helpful to show it on devtools. Of course, I still need to refer to your comments.

@InMatrix @jacob314 Thanks for the suggestion. After confirming that the code can be merged, I will fix code problem uniformly

mengdouer avatar Jul 28 '22 03:07 mengdouer

Can we add a tab to the right of the Details Trees so that when the component is selected we can view the screenshot without additional clicks.

This sounds reasonable. Could you prototype it and post a video?

Also, how performant is taking screenshots for individual widgets? For example, when the user quickly cycle through a bunch of widgets in the tree view, how long does it take for each screenshot to load?

InMatrix avatar Jul 28 '22 04:07 InMatrix

Can we add a tab to the right of the Details Trees so that when the component is selected we can view the screenshot without additional clicks.

This sounds reasonable. Could you prototype it and post a video?

Also, how performant is taking screenshots for individual widgets? For example, when the user quickly cycle through a bunch of widgets in the tree view, how long does it take for each screenshot to load?

OK I'll implement a prototype later. In terms of performance, there is no test yet. I think @jacob314 may know more about it, and we can listen to his opinions and I will do some tests later too

mengdouer avatar Jul 28 '22 15:07 mengdouer

@mengdouer Is this PR still on your radar?

Hixie avatar Oct 18 '22 23:10 Hixie

@mengdouer thanks for your contribution! I'm going to close this PR since it appears work here has stalled, but please do not hesitate to reopen it if you would like to resume work. You may also wish to file a bug with this proposal and link to this PR from the bug in case someone else would like to take over!

Hixie avatar Feb 14 '23 23:02 Hixie