address code review: simplify copyable text and tooltip implementation

canonical · Feb 3, 2025 · 2f58580 · 2f58580
1 parent e4e0678
commit 2f58580
Show file tree

Hide file tree

Showing 5 changed files with 24 additions and 71 deletions.
diff --git a/src/client/gui/lib/copyable_text.dart b/src/client/gui/lib/copyable_text.dart
@@ -1,5 +1,6 @@
 import 'package:flutter/material.dart' hide Tooltip;
 import 'package:flutter/services.dart';
+
 import 'tooltip.dart';
 
 class CopyableText extends StatefulWidget {
@@ -13,20 +14,7 @@ class CopyableText extends StatefulWidget {
 }
 
 class _CopyableTextState extends State<CopyableText> {
-  bool _copied = false;
-  bool get _isCopyable => widget.text != '-';
-
-  void _copyToClipboard() async {
-    if (!_isCopyable) return;
-    await Clipboard.setData(ClipboardData(text: widget.text));
-    setState(() => _copied = true);
-  }
-
-  void _resetCopied() {
-    if (_copied) {
-      setState(() => _copied = false);
-    }
-  }
+  var _copied = false;
 
   @override
   Widget build(BuildContext context) {
@@ -37,13 +25,17 @@ class _CopyableTextState extends State<CopyableText> {
       overflow: TextOverflow.ellipsis,
     );
 
-    if (!_isCopyable) return text;
+    // if there is no value, display "-"
+    if (widget.text == '-') return text;
 
     return MouseRegion(
       cursor: SystemMouseCursors.click,
-      onExit: (_) => _resetCopied(),
+      onExit: (_) => setState(() => _copied = false),
       child: GestureDetector(
-        onTap: _copyToClipboard,
+        onTap: () async {
+          await Clipboard.setData(ClipboardData(text: widget.text));
+          setState(() => _copied = true);
+        },
         child: Tooltip(
           message: _copied ? 'Copied' : 'Click to copy',
           child: text,

diff --git a/src/client/gui/lib/tooltip.dart b/src/client/gui/lib/tooltip.dart
@@ -1,6 +1,6 @@
 import 'package:flutter/material.dart' as fl;
 
-class Tooltip extends fl.StatefulWidget {
+class Tooltip extends fl.StatelessWidget {
   final fl.Widget child;
   final String message;
   final bool visible;
@@ -12,40 +12,19 @@ class Tooltip extends fl.StatefulWidget {
     this.visible = true,
   });
 
-  @override
-  fl.State<Tooltip> createState() => _TooltipState();
-}
-
-class _TooltipState extends fl.State<Tooltip> {
-  final _key = fl.GlobalKey<fl.TooltipState>();
-  bool _forceShow = false;
-
-  @override
-  void didUpdateWidget(Tooltip oldWidget) {
-    super.didUpdateWidget(oldWidget);
-    if (oldWidget.message != widget.message) {
-      setState(() => _forceShow = true);
-      Future.delayed(const Duration(milliseconds: 1), () {
-        if (mounted) {
-          setState(() => _forceShow = false);
-        }
-      });
-    }
-  }
-
   @override
   fl.Widget build(fl.BuildContext context) {
     return fl.TooltipVisibility(
-      visible: widget.visible,
+      visible: visible,
       child: fl.Tooltip(
-        key: _forceShow ? _key : null,
-        message: widget.message,
+        key: fl.Key(message),
+        message: message,
         textAlign: fl.TextAlign.center,
         decoration: fl.BoxDecoration(
           color: const fl.Color(0xff111111),
           borderRadius: fl.BorderRadius.circular(2),
         ),
-        child: widget.child,
+        child: child,
       ),
     );
   }

diff --git a/src/client/gui/lib/vm_details/ip_addresses.dart b/src/client/gui/lib/vm_details/ip_addresses.dart
@@ -1,30 +1,21 @@
 import 'package:flutter/material.dart' hide Tooltip;
-import '../copyable_text.dart';
 
+import '../copyable_text.dart';
 import '../extensions.dart';
 import '../tooltip.dart';
 
 class IpAddresses extends StatelessWidget {
   final Iterable<String> ips;
-  final bool copyable;
 
-  const IpAddresses(this.ips, {this.copyable = false, super.key});
+  const IpAddresses(this.ips, {super.key});
 
   @override
   Widget build(BuildContext context) {
     final firstIp = ips.firstOrNull ?? '-';
     final restIps = ips.skip(1).toList();
 
     return Row(children: [
-      Expanded(
-        child: copyable
-            ? CopyableText(firstIp)
-            : Tooltip(
-                message: firstIp,
-                child:
-                    Text(firstIp.nonBreaking, overflow: TextOverflow.ellipsis),
-              ),
-      ),
+      Expanded(child: CopyableText(firstIp)),
       if (restIps.isNotEmpty)
         Badge.count(
           count: restIps.length,

diff --git a/src/client/gui/lib/vm_details/vm_details_general.dart b/src/client/gui/lib/vm_details/vm_details_general.dart
@@ -1,18 +1,17 @@
 import 'package:basics/basics.dart';
-import 'package:flutter/material.dart' hide Tooltip;
+import 'package:flutter/material.dart';
 import 'package:flutter_riverpod/flutter_riverpod.dart';
 import 'package:intl/intl.dart';
-import 'package:flutter/services.dart';
 
+import '../copyable_text.dart';
 import '../extensions.dart';
 import '../providers.dart';
+import '../tooltip.dart';
 import 'cpu_sparkline.dart';
 import 'memory_usage.dart';
 import 'vm_action_buttons.dart';
 import 'vm_details.dart';
 import 'vm_status_icon.dart';
-import '../tooltip.dart';
-import '../copyable_text.dart';
 
 class VmDetailsHeader extends ConsumerWidget {
   final String name;

diff --git a/src/client/gui/lib/vm_table/vm_table_headers.dart b/src/client/gui/lib/vm_table/vm_table_headers.dart
@@ -3,6 +3,7 @@ import 'package:built_collection/built_collection.dart';
 import 'package:flutter/material.dart' hide Tooltip;
 import 'package:flutter_riverpod/flutter_riverpod.dart';
 
+import '../copyable_text.dart';
 import '../extensions.dart';
 import '../providers.dart';
 import '../sidebar.dart';
@@ -14,7 +15,6 @@ import '../vm_details/vm_status_icon.dart';
 import 'search_box.dart';
 import 'table.dart';
 import 'vms.dart';
-import '../copyable_text.dart';
 
 final headers = <TableHeader<VmInfo>>[
   TableHeader(
@@ -73,28 +73,20 @@ final headers = <TableHeader<VmInfo>>[
     minWidth: 70,
     cellBuilder: (info) {
       final image = info.instanceInfo.currentRelease;
-      return CopyableText(
-        image.isNotBlank ? image.nonBreaking : '-',
-      );
+      return CopyableText(image.isNotBlank ? image.nonBreaking : '-');
     },
   ),
   TableHeader(
     name: 'PRIVATE IP',
     width: 140,
     minWidth: 100,
-    cellBuilder: (info) => IpAddresses(
-      info.instanceInfo.ipv4.take(1),
-      copyable: true,
-    ),
+    cellBuilder: (info) => IpAddresses(info.instanceInfo.ipv4.take(1)),
   ),
   TableHeader(
     name: 'PUBLIC IP',
     width: 140,
     minWidth: 100,
-    cellBuilder: (info) => IpAddresses(
-      info.instanceInfo.ipv4.skip(1),
-      copyable: true,
-    ),
+    cellBuilder: (info) => IpAddresses(info.instanceInfo.ipv4.skip(1)),
   ),
 ];