Repository navigation
Combine resume / pause buttons in the debugger controls - #8858
bwilkerson wants to merge 3 commits into
Conversation
| [#8849](https://github.com/flutter/devtools/pull/8849) | ||
| * Either the 'pause' or the 'resume' buttons is displayed, | ||
| never both. - | ||
| [#4924](https://github.com/flutter/devtools/issues/4924) |
There was a problem hiding this comment.
Nit: Please link the PR instead of the issue that this fixes.
Uber nit on verbiage: entries should be phrased in the past tense (e.g. "Added XYZ" instead of "Add XYZ").
kenzieschmoll
left a comment
kenzieschmoll
left a comment
There was a problem hiding this comment.
LGTM with a nit on the release note entry to match the preferred style laid out in the release notes README.md
| @@ -93,27 +93,27 @@ class _DebuggingControlsState extends State<DebuggingControls> | |||
| final isSystemIsolate = controller.isSystemIsolate; | |||
| return RoundedButtonGroup( | |||
There was a problem hiding this comment.
Now that we only have one item in items, I don't think we need the RoundedButtonGroup (since it's intended for a group of buttons). I think we probably want to use DevToolButton here (though we might need to add a rounded option which would match the UI of `RoundedButtonGroup). Adding @kenzieschmoll if she has other thoughts!
There was a problem hiding this comment.
+1 we can use DevToolsButton instead. One clean up could be to change this method to return a ternary:
return isPaused ? DevToolsButton(...) : DevToolsButton(...);;
|
Looks like there are also some failing tests that need to be updated: |
|
Thanks for the feedback. The test failure is probably more than I can do in a codespace, so if one of you wants to finish this off I'm fine with that. Otherwise I'll just close it without merging it. |
|
closing in favor of #9095 |
3 participants
This fixes #4924 by showing only one button at any given time.