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.

Sign up for GitHub

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

Add floating debugger controls when app is paused #2676

Merged
merged 4 commits into from
Feb 4, 2021
+321 −44
Merged

Add floating debugger controls when app is paused #2676

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0646f18
Add floating debugger controls when app is paused
kenzieschmoll Feb 4, 2021
18f9c8a
remove prints
kenzieschmoll Feb 4, 2021
ed3306a
remove keys for testing
kenzieschmoll Feb 4, 2021
acc0e65
use animated opacity
kenzieschmoll Feb 4, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions packages/devtools_app/lib/src/app.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,7 @@ class _AlternateCheckedModeBanner extends StatelessWidget {
class OpenAboutAction extends StatelessWidget {
@override
Widget build(BuildContext context) {
return ActionButton(
return DevToolsTooltip(
tooltip: 'About DevTools',
child: InkWell(
onTap: () async {
Expand All @@ -390,7 +390,7 @@ class OpenAboutAction extends StatelessWidget {
class OpenSettingsAction extends StatelessWidget {
@override
Widget build(BuildContext context) {
return ActionButton(
return DevToolsTooltip(
tooltip: 'Settings',
child: InkWell(
onTap: () async {
Expand Down
37 changes: 35 additions & 2 deletions packages/devtools_app/lib/src/common_widgets.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -568,8 +568,8 @@ class Badge extends StatelessWidget {

/// A widget, commonly used for icon buttons, that provides a tooltip with a
/// common delay before the tooltip is shown.
class ActionButton extends StatelessWidget {
const ActionButton({
class DevToolsTooltip extends StatelessWidget {
const DevToolsTooltip({
@required this.tooltip,
@required this.child,
});
Expand Down Expand Up @@ -789,6 +789,39 @@ Widget inputDecorationSuffixButton(IconData icon, VoidCallback onPressed) {
);
}

class OutlinedRowGroup extends StatelessWidget {
const OutlinedRowGroup({@required this.children, this.borderColor});

final List<Widget> children;

final Color borderColor;

@override
Widget build(BuildContext context) {
final color = borderColor ?? Theme.of(context).focusColor;
final childrenWithOutlines = <Widget>[];
for (int i = 0; i < children.length; i++) {
childrenWithOutlines.addAll([
Container(
decoration: BoxDecoration(
border: Border(
left: i == 0 ? BorderSide(color: color) : BorderSide.none,
Copy link
Contributor

Choose a reason for hiding this comment

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

why is only the left side special cased and not the right side.?

Copy link
Member Author

Choose a reason for hiding this comment

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

We do this to make sure there are not any double borders. The right side of the previous element will serve as the left border for the next element (for all except the first since the first does not have a previous element)

right: BorderSide(color: color),
top: BorderSide(color: color),
bottom: BorderSide(color: color),
),
),
child: children[i],
),
]);
}
return Row(
mainAxisSize: MainAxisSize.min,
children: childrenWithOutlines,
);
}
}

class OutlineDecoration extends StatelessWidget {
const OutlineDecoration({Key key, this.child}) : super(key: key);

Expand Down
2 changes: 2 additions & 0 deletions packages/devtools_app/lib/src/debugger/debugger_controller.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,7 @@ class DebuggerController extends DisposableController
if (isolate.pauseEvent != null &&
isolate.pauseEvent.kind != EventKind.kResume) {
_lastEvent = isolate.pauseEvent;
print('pausing from switchToIsolate');
await _pause(true, pauseEvent: isolate.pauseEvent);
}

Expand Down Expand Up @@ -438,6 +439,7 @@ class DebuggerController extends DisposableController
// Any event we receive here indicates that any resume/step request has been
// processed.
_resuming.value = false;
print('pausing from kPause debug event');
_pause(true, pauseEvent: event);
break;
// TODO(djshuckerow): switch the _breakpoints notifier to a 'ListNotifier'
Expand Down
72 changes: 71 additions & 1 deletion packages/devtools_app/lib/src/debugger/debugger_screen.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ class DebuggerScreenBodyState extends State<DebuggerScreenBody>
builder: (context, breakpoints, _) {
return Row(children: [
BreakpointsCountBadge(breakpoints: breakpoints),
ActionButton(
DevToolsTooltip(
child: ToolbarAction(
icon: Icons.delete,
onPressed:
Expand Down Expand Up @@ -370,3 +370,73 @@ class _DebuggerStatusState extends State<DebuggerStatus> with AutoDisposeMixin {
return 'paused$reason$fileName $pos';
}
}

class FloatingDebuggerControls extends StatelessWidget {
@visibleForTesting
static const debuggerControlsKey = Key('debugger controls');

@visibleForTesting
static const debuggerControlsResumeKey = Key('debugger controls - resume');

@visibleForTesting
static const debuggerControlsStepKey = Key('debugger controls - step');

@override
Widget build(BuildContext context) {
final controller = Provider.of<DebuggerController>(context);
return ValueListenableBuilder<bool>(
valueListenable: controller.isPaused,
builder: (context, paused, _) {
if (!paused) return const SizedBox();
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: rather than having this become a sized box when not paused consider making the ui unclickable when not paused and providing an animated transition showing and hiding the ui.

Copy link
Member Author

Choose a reason for hiding this comment

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

good idea. done - see pr description for gif

return Container(
key: debuggerControlsKey,
color: devtoolsWarning,
height: defaultButtonHeight,
child: OutlinedRowGroup(
// Default focus color for the light theme - since the background
// color of the controls [devtoolsWarning] is the same for both
// themes, we will use the same border color.
borderColor: Colors.black.withOpacity(0.12),
children: [
Container(
height: defaultButtonHeight,
alignment: Alignment.centerLeft,
padding: const EdgeInsets.symmetric(
horizontal: defaultSpacing,
),
child: const Text(
'Main isolate is paused in the debugger',
style: TextStyle(color: Colors.black),
),
),
DevToolsTooltip(
tooltip: 'Resume',
child: TextButton(
key: debuggerControlsResumeKey,
onPressed: controller.resume,
child: const Icon(
Icons.play_arrow,
color: Colors.green,
size: defaultIconSize,
),
),
),
DevToolsTooltip(
tooltip: 'Step over',
child: TextButton(
key: debuggerControlsStepKey,
onPressed: controller.stepOver,
child: const Icon(
Icons.keyboard_arrow_right,
color: Colors.black,
size: defaultIconSize,
),
),
),
],
),
);
},
);
}
}
70 changes: 38 additions & 32 deletions packages/devtools_app/lib/src/scaffold.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import 'common_widgets.dart';
import 'config_specific/drag_and_drop/drag_and_drop.dart';
import 'config_specific/ide_theme/ide_theme.dart';
import 'config_specific/import_export/import_export.dart';
import 'debugger/debugger_screen.dart';
import 'framework_controller.dart';
import 'globals.dart';
import 'notifications.dart';
Expand Down Expand Up @@ -119,7 +120,7 @@ class DevToolsScaffoldState extends State<DevToolsScaffold>
/// coordinate their animation when the tab selection changes.
TabController _tabController;

final ValueNotifier<Screen> _currentScreen = ValueNotifier(null);
Screen _currentScreen;

ImportController _importController;

Expand Down Expand Up @@ -182,7 +183,6 @@ class DevToolsScaffoldState extends State<DevToolsScaffold>
@override
void dispose() {
_tabController?.dispose();
_currentScreen?.dispose();
_connectVmSubscription?.cancel();
_showPageSubscription?.cancel();

Expand All @@ -201,12 +201,14 @@ class DevToolsScaffoldState extends State<DevToolsScaffold>
}
}

_currentScreen.value = widget.tabs[_tabController.index];
_currentScreen = widget.tabs[_tabController.index];
_tabController.addListener(() {
final screen = widget.tabs[_tabController.index];

if (_currentScreen.value != screen) {
_currentScreen.value = screen;
if (_currentScreen != screen) {
setState(() {
_currentScreen = screen;
});

// Send the page change info to the framework controller (it can then
// send it on to the devtools server, if one is connected).
Expand All @@ -231,7 +233,7 @@ class DevToolsScaffoldState extends State<DevToolsScaffold>

// Broadcast the initial page.
frameworkController.notifyPageChange(
PageChangeEvent(_currentScreen.value.screenId, widget.embed),
PageChangeEvent(_currentScreen.screenId, widget.embed),
);
}

Expand Down Expand Up @@ -301,31 +303,35 @@ class DevToolsScaffoldState extends State<DevToolsScaffold>

final title = widget.title ?? generateDevToolsTitle();
final theme = Theme.of(context);
// TODO(issues/2547) - Remove.
// ignore: deprecated_member_use
return ValueListenableProvider.value(
value: _currentScreen,
child: Provider<BannerMessagesController>(
create: (_) => BannerMessagesController(),
child: DragAndDrop(
handleDrop: _importController.importData,
child: Title(
title: title,
// Color is a required parameter but the color only appears to
// matter on Android and we do not care about Android.
// Using theme.primaryColor matches the default behavior of the
// title used by [WidgetsApp].
color: theme.primaryColor,
child: Scaffold(
appBar: widget.embed ? null : _buildAppBar(title),
body: TabBarView(
physics: defaultTabBarViewPhysics,
controller: _tabController,
children: tabBodies,
),
bottomNavigationBar:
widget.embed ? null : _buildStatusLine(context),
return Provider<BannerMessagesController>(
create: (_) => BannerMessagesController(),
child: DragAndDrop(
handleDrop: _importController.importData,
child: Title(
title: title,
// Color is a required parameter but the color only appears to
// matter on Android and we do not care about Android.
// Using theme.primaryColor matches the default behavior of the
// title used by [WidgetsApp].
color: theme.primaryColor,
child: Scaffold(
appBar: widget.embed ? null : _buildAppBar(title),
body: Stack(
children: [
TabBarView(
physics: defaultTabBarViewPhysics,
controller: _tabController,
children: tabBodies,
),
if (serviceManager.hasConnection &&
_currentScreen.screenId != DebuggerScreen.id)
Container(
alignment: Alignment.topCenter,
child: FloatingDebuggerControls(),
),
],
),
bottomNavigationBar: widget.embed ? null : _buildStatusLine(),
),
),
),
Expand Down Expand Up @@ -400,7 +406,7 @@ class DevToolsScaffoldState extends State<DevToolsScaffold>
);
}

Widget _buildStatusLine(BuildContext context) {
Widget _buildStatusLine() {
const appPadding = DevToolsScaffold.appPadding;

return Container(
Expand All @@ -415,7 +421,7 @@ class DevToolsScaffoldState extends State<DevToolsScaffold>
right: appPadding.right,
bottom: appPadding.bottom,
),
child: StatusLine(),
child: StatusLine(_currentScreen),
),
],
),
Expand Down
9 changes: 5 additions & 4 deletions packages/devtools_app/lib/src/status_line.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import 'dart:async';

import 'package:flutter/material.dart';
import 'package:pedantic/pedantic.dart';
import 'package:provider/provider.dart';
import 'package:vm_service/vm_service.dart';

import '../devtools.dart' as devtools;
Expand All @@ -26,12 +25,14 @@ const statusLineHeight = 24.0;
/// This displays information global to the application, as well as gives pages
/// a mechanism to display page-specific information.
class StatusLine extends StatelessWidget {
const StatusLine(this.currentScreen);

final Screen currentScreen;

@override
Widget build(BuildContext context) {
final textTheme = Theme.of(context).textTheme;

final Screen currentScreen = Provider.of<Screen>(context);

final List<Widget> children = [];

// Have an area for page specific help (always docked to the left).
Expand Down Expand Up @@ -211,7 +212,7 @@ class StatusLine extends StatelessWidget {
},
),
const SizedBox(width: denseSpacing),
ActionButton(
DevToolsTooltip(
tooltip: 'Device Info',
child: InkWell(
onTap: () async {
Expand Down
4 changes: 2 additions & 2 deletions packages/devtools_app/lib/src/ui/service_extension_widgets.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ class HotReloadButton extends StatelessWidget {
Widget build(BuildContext context) {
// TODO(devoncarew): Show as disabled when reload service calls are in progress.

return ActionButton(
return DevToolsTooltip(
tooltip: 'Hot reload',
child: _RegisteredServiceExtensionButton._(
serviceDescription: hotReload,
Expand All @@ -200,7 +200,7 @@ class HotRestartButton extends StatelessWidget {
Widget build(BuildContext context) {
// TODO(devoncarew): Show as disabled when reload service calls are in progress.

return ActionButton(
return DevToolsTooltip(
tooltip: 'Hot restart',
child: _RegisteredServiceExtensionButton._(
serviceDescription: hotRestart,
Expand Down
Loading