From fa20dda35e112d22a496829eb771bdf31e905674 Mon Sep 17 00:00:00 2001
From: lasse-numerous <lasse.thomsen@numerous.com>
Date: Mon, 16 Dec 2024 21:15:35 +0100
Subject: [PATCH] refactor(ui): Simplify button and progress bar components;
 enhance task modal layout

- Removed Tooltip from Button component and added tooltip as a button attribute for better accessibility.
- Updated ProgressBar component to improve structure and styling, including a new container for the progress track.
- Refactored TaskButton to use a wrapper class for better styling and layout.
- Enhanced TaskModal layout with improved error handling display and integrated CheckBox and Button components for better user interaction.
- Cleaned up CSS styles for consistency and removed unused styles.
---
 js/src/components/ui/Button.tsx        |   8 +-
 js/src/components/ui/ProgressBar.tsx   |  14 +-
 js/src/components/ui/TaskButton.tsx    |   6 +-
 js/src/components/ui/TaskModal.tsx     | 112 ++++++-------
 js/src/css/styles.css                  | 207 +++++++------------------
 python/examples/marimo/numerous/app.py |   3 +-
 6 files changed, 131 insertions(+), 219 deletions(-)

diff --git a/js/src/components/ui/Button.tsx b/js/src/components/ui/Button.tsx
index f66f04e..d0711a5 100644
--- a/js/src/components/ui/Button.tsx
+++ b/js/src/components/ui/Button.tsx
@@ -1,5 +1,4 @@
 import * as React from "react";
-import { Tooltip } from './Tooltip';
 
 interface ButtonProps {
     label: string;
@@ -18,17 +17,12 @@ export function Button({
 }: ButtonProps) {
     return (
         <div className="button-container">
-            {label && (
-                <label className="button-label">
-                    <span>{label}</span>
-                    {tooltip && <Tooltip tooltip={tooltip} />}
-                </label>
-            )}
             <button 
                 className="widget-button"
                 onClick={onClick}
                 disabled={disabled}
                 aria-pressed={value}
+                title={tooltip}
             >
                 {label}
             </button>
diff --git a/js/src/components/ui/ProgressBar.tsx b/js/src/components/ui/ProgressBar.tsx
index 713da08..78d181f 100644
--- a/js/src/components/ui/ProgressBar.tsx
+++ b/js/src/components/ui/ProgressBar.tsx
@@ -19,17 +19,21 @@ export function ProgressBar({ value, uiLabel, uiTooltip }: ProgressBarProps) {
                     {uiTooltip && <Tooltip tooltip={uiTooltip} />}
                 </div>
             )}
-            <div className="progress-track">
+            <div className="progress-track-container">
                 <div 
-                    className="progress-fill"
-                    style={{ width: `${clampedValue}%` }}
+                    className="progress-track"
                     role="progressbar"
                     aria-valuenow={clampedValue}
                     aria-valuemin={0}
                     aria-valuemax={100}
-                />
+                >
+                    <div 
+                        className="progress-fill"
+                        style={{ width: `${clampedValue}%` }}
+                    />
+                </div>
+                <div className="progress-value">{Math.round(clampedValue)}%</div>
             </div>
-            <div className="progress-value">{Math.round(clampedValue)}%</div>
         </div>
     );
 }
diff --git a/js/src/components/ui/TaskButton.tsx b/js/src/components/ui/TaskButton.tsx
index 05c751c..53189a9 100644
--- a/js/src/components/ui/TaskButton.tsx
+++ b/js/src/components/ui/TaskButton.tsx
@@ -78,7 +78,7 @@ export function TaskButton({
     };
 
     return (
-        <div className="task-button-container">
+        <div className="task-button-wrapper">
             <button
                 className={`task-button ${getButtonState()} ${isHovering ? 'hovering' : ''}`}
                 onClick={handleClick}
@@ -151,8 +151,8 @@ export function TaskButton({
                 onClick={handleExpand}
                 disabled={isDisabled}
             >
-                <svg viewBox="0 0 24 24" fill="currentColor" className="expand-icon" width="16" height="16">
-                    <path d="M7 14H5v5h5v-2H7v-3zm-2-4h2V7h3V5H5v5zm12 7h-3v2h5v-5h-2v3zM14 5v2h3v3h2V5h-5z"/>
+                <svg viewBox="0 0 24 24" fill="currentColor" className="expand-icon">
+                    <path d="M15.5 14h-.79l-.28-.27C15.41 12.59 16 11.11 16 9.5 16 5.91 13.09 3 9.5 3S3 5.91 3 9.5 5.91 16 9.5 16c1.61 0 3.09-.59 4.23-1.57l.27.28v.79l5 4.99L20.49 19l-4.99-5zm-6 0C7.01 14 5 11.99 5 9.5S7.01 5 9.5 5 14 7.01 14 9.5 11.99 14 9.5 14z"/>
                 </svg>
             </button>
 
diff --git a/js/src/components/ui/TaskModal.tsx b/js/src/components/ui/TaskModal.tsx
index fcfb79a..8925517 100644
--- a/js/src/components/ui/TaskModal.tsx
+++ b/js/src/components/ui/TaskModal.tsx
@@ -1,5 +1,7 @@
 import * as React from 'react';
 import { ProgressBar } from './ProgressBar';
+import { Button } from './Button';
+import { CheckBox } from './CheckBox';
 
 interface LogEntry {
     timestamp: string;
@@ -75,76 +77,76 @@ export function TaskDetailModal({
     };
 
     return (
-        <div className="modal-overlay fixed inset-0 bg-black bg-opacity-50 flex items-center justify-center z-50 p-4" 
+        <div 
+            className="fixed inset-0 bg-black bg-opacity-50 flex items-center justify-center z-50" 
             onClick={handleOverlayClick}
         >
-            <div className="modal-container bg-white rounded-xl shadow-2xl w-1/2 min-w-[500px] max-w-2xl relative z-50">
-                <div className="task-detail-modal p-8 flex flex-col items-center space-y-6">
-                    <div className="w-3/4 flex justify-center">
+            <div className="bg-white rounded-lg shadow-xl w-full max-w-2xl mx-4 max-h-[90vh] flex flex-col border-2 border-gray-300">
+                <div className="p-6 flex flex-col h-full">
+                    <h2 className="text-xl font-semibold text-gray-900 mb-4 text-center">
+                        {taskName}
+                    </h2>
+
+                    <div className="w-full mb-6">
                         <ProgressBar value={progress * 100} />
                     </div>
 
                     {error && error.message && (
-                        <div className="w-full">
-                            <div className="error-container bg-red-50 rounded-lg p-4 border border-red-200">
-                                {error.timestamp && (
-                                    <p className="text-red-500 text-xs mb-2">
-                                        {formatTimestamp(error.timestamp)}
+                        <div className="bg-red-50 rounded-lg p-4 border border-red-200 mb-6">
+                            {error.timestamp && (
+                                <p className="text-red-500 text-xs mb-2">
+                                    {formatTimestamp(error.timestamp)}
+                                </p>
+                            )}
+                            <div className="flex items-start gap-3">
+                                <svg className="w-6 h-6 text-red-600 mt-0.5" viewBox="0 0 20 20" fill="currentColor">
+                                    <path fillRule="evenodd" d="M10 18a8 8 0 100-16 8 8 0 000 16zM8.28 7.22a.75.75 0 00-1.06 1.06L8.94 10l-1.72 1.72a.75.75 0 101.06 1.06L10 11.06l1.72 1.72a.75.75 0 101.06-1.06L11.06 10l1.72-1.72a.75.75 0 00-1.06-1.06L10 8.94 8.28 7.22z" clipRule="evenodd" />
+                                </svg>
+                                <div className="flex-1">
+                                    <p className="text-red-700 font-medium text-sm">
+                                        {typeof error.message === 'object' 
+                                            ? JSON.stringify(error.message) 
+                                            : error.message}
                                     </p>
-                                )}
-                                <div className="flex items-start gap-3">
-                                    <svg className="w-8 h-8 text-red-600 mt-0.5" viewBox="0 0 20 20" fill="currentColor">
-                                        <path fillRule="evenodd" d="M10 18a8 8 0 100-16 8 8 0 000 16zM8.28 7.22a.75.75 0 00-1.06 1.06L8.94 10l-1.72 1.72a.75.75 0 101.06 1.06L10 11.06l1.72 1.72a.75.75 0 101.06-1.06L11.06 10l1.72-1.72a.75.75 0 00-1.06-1.06L10 8.94 8.28 7.22z" clipRule="evenodd" />
-                                    </svg>
-                                    <div className="flex-1">
-                                        <p className="text-red-700 font-bold text-xl">
-                                            {typeof error.message === 'object' 
-                                                ? JSON.stringify(error.message) 
-                                                : error.message}
-                                        </p>
-                                        {error.traceback && (
-                                            <pre className="mt-2 text-red-600 text-sm font-mono whitespace-pre-wrap overflow-x-auto">
-                                                {error.traceback}
-                                            </pre>
-                                        )}
-                                    </div>
+                                    {error.traceback && (
+                                        <pre className="mt-2 text-red-600 text-xs font-mono whitespace-pre-wrap overflow-x-auto">
+                                            {error.traceback}
+                                        </pre>
+                                    )}
                                 </div>
                             </div>
                         </div>
                     )}
 
-                    <div className="w-full">
-                        <div 
-                            ref={logsContainerRef}
-                            className="logs-container bg-gray-50 rounded-lg p-4 max-h-[300px] overflow-y-auto border border-gray-100"
-                        >
-                            {logs.map(([timestamp, type, source, message], index) => (
-                                <div key={index} className="log-entry py-1 font-mono text-sm flex gap-2">
-                                    <span className="text-gray-400">{formatTimestamp(timestamp)}</span>
-                                    <span className={`font-medium ${getLogTypeColor(type)}`}>[{type}]</span>
-                                    <span className="text-gray-500">{source}:</span>
-                                    <span className="text-gray-700">{message}</span>
-                                </div>
-                            ))}
-                        </div>
+                    <div 
+                        ref={logsContainerRef}
+                        className="bg-gray-50 rounded-lg p-4 overflow-y-auto border border-gray-200 mb-6"
+                        style={{
+                            height: '500px',
+                            lineHeight: '25px'
+                        }}
+                    >
+                        {logs.slice(-20).map(([timestamp, type, source, message], index) => (
+                            <div key={index} className="py-1 font-mono text-xs flex gap-2 h-[25px] items-center">
+                                <span className="text-gray-400">{formatTimestamp(timestamp)}</span>
+                                <span className={`font-medium ${getLogTypeColor(type)}`}>[{type}]</span>
+                                <span className="text-gray-500">{source}:</span>
+                                <span className="text-gray-700">{message}</span>
+                            </div>
+                        ))}
                     </div>
 
-                    <div className="flex items-center space-x-6">
-                        <label className="flex items-center space-x-2 text-sm text-gray-600">
-                            <input
-                                type="checkbox"
-                                checked={isAutoScrollEnabled}
-                                onChange={(e) => setIsAutoScrollEnabled(e.target.checked)}
-                                className="rounded text-blue-500 focus:ring-blue-500"
-                            />
-                            <span>Track updates</span>
-                        </label>
-                        <button 
-                            className="px-8 py-2.5 bg-blue-500 hover:bg-blue-600 rounded-lg transition-colors duration-200 font-medium text-white shadow-sm"
+                    <div className="flex items-center justify-between mt-auto">
+                        <CheckBox
+                            value={isAutoScrollEnabled}
+                            onChange={(checked) => setIsAutoScrollEnabled(checked)}
+                            uiLabel="Auto-scroll"
+                        />
+                        <Button 
+                            label="Close"
+                            tooltip="Close the task details"
                             onClick={onClose}
-                        >
-                            Close
-                        </button>
+                        />
                     </div>
                 </div>
             </div>
diff --git a/js/src/css/styles.css b/js/src/css/styles.css
index 8bd2328..01047f3 100644
--- a/js/src/css/styles.css
+++ b/js/src/css/styles.css
@@ -726,15 +726,12 @@ select::-ms-expand {
   flex-direction: column;
   gap: 8px;
   width: 100%;
-  max-width: 300px;
 }
 
-.progress-label {
+.progress-track-container {
   display: flex;
   align-items: center;
-  gap: 4px;
-  color: var(--ui-widget-primary-text);
-  font-size: 14px;
+  gap: 8px;
 }
 
 .progress-track {
@@ -1230,181 +1227,95 @@ select::-ms-expand {
 }
 
 .expand-button {
+  position: absolute;
+  top: -4px;
+  right: -16px;
   padding: 4px;
-  border: none;
   background: transparent;
+  border: none;
+  border-radius: 4px;
   cursor: pointer;
-  opacity: 0.6;
-  transition: opacity 0.2s;
-  color: var(--ui-widget-secondary-text);
-}
-
-.expand-button:hover:not(:disabled) {
-  opacity: 1;
+  display: flex;
+  align-items: center;
+  justify-content: center;
+  z-index: 1;
 }
 
-.expand-button:disabled {
-  opacity: 0.3;
-  cursor: not-allowed;
+.expand-button:hover {
+  background-color: transparent;
 }
 
 .expand-icon {
-  width: 16px;
-  height: 16px;
-}
-
-.text-lg {
-  font-size: 1.125rem;
-  line-height: 1.75rem;
-}
-
-.font-semibold {
-  font-weight: 600;
-}
-
-.text-sm {
-  font-size: 0.875rem;
-  line-height: 1.25rem;
-}
-
-.text-gray-500 {
+  width: 12px;
+  height: 12px;
   color: var(--ui-widget-secondary-text);
+  opacity: 0.6;
 }
 
-[data-radix-popper-content-wrapper] {
-  z-index: 9999 !important;
-}
-
-[data-radix-dialog-overlay] {
-  position: fixed;
-  inset: 0;
-  background-color: rgba(0, 0, 0, 0.5);
-  z-index: 9998;
-}
-
-[data-radix-dialog-content] {
-  position: fixed;
-  left: 50%;
-  top: 50%;
-  transform: translate(-50%, -50%);
-  z-index: 9999;
-}
-
-.number-input-container.invalid input[type="number"] {
-  border-color: red;
-}
-
-.number-input-container.invalid .input-wrapper {
-  border-color: #ef4444;
+.expand-button:hover .expand-icon {
+  opacity: 1;
 }
 
-.validation-message {
-  position: absolute;
-  bottom: 100%;
-  left: 50%;
-  transform: translateX(-50%);
-  background-color: #fff;
-  border: 1px solid #ef4444;
-  padding: 4px 8px;
+.magnify-button {
+  padding: 4px;
+  background: var(--ui-widget-secondary-background);
+  border: 1px solid var(--ui-widget-border-color);
   border-radius: 4px;
-  box-shadow: 0 2px 4px rgba(0, 0, 0, 0.1);
-  z-index: 10;
-  white-space: nowrap;
-  margin-bottom: 4px;
-  color: #ef4444;
-  font-size: 12px;
-  display: none;
-}
-
-.number-input-container.invalid:hover .validation-message {
-  display: block;
-}
-
-.number-input-container.invalid .input-wrapper {
-  border-color: #ef4444;
-}
-
-.number-input-container.invalid input[type="number"] {
-  color: #ef4444;
-}
-
-.widget-container {
+  cursor: pointer;
   display: flex;
-  width: 100%;
-  margin: 0;
-  margin-top: 0px;
-  margin-bottom: 0px;
-  margin-left: 0px;
-  margin-right: 0px;
-  padding: 0px;
+  align-items: center;
+  justify-content: center;
 }
 
-.widget-container.hidden {
-  display: none;
+.magnify-button:hover {
+  background-color: var(--ui-widget-hover-background);
 }
 
-.file-loader-container {
-    display: inline-block;
+.magnify-icon {
+  color: var(--ui-widget-secondary-text);
 }
 
-.file-loader-button {
-    display: flex;
-    align-items: center;
-    gap: 0.5rem;
-    padding: 0.5rem 1rem;
-    border: 1px solid #ccc;
-    border-radius: 4px;
-    background: #f8f8f8;
-    cursor: pointer;
+.task-button-wrapper {
+  position: relative;
+  display: inline-block;
+  width: fit-content;
+  /* background: rgba(255, 0, 0, 0.1); /* temporary debug color */
 }
 
-.file-loader-button:hover {
-    background: #eee;
+.task-button {
+  position: relative;
+  /* background: rgba(0, 255, 0, 0.1); /* temporary debug color */
 }
 
-.widget-container-bordered {
-  border: 1px solid #3d3d3d;
+.expand-button {
+  position: absolute;
+  top: -4px;
+  right: -20px;
+  padding: 4px;
+  background: transparent;
+  border: none;
   border-radius: 4px;
-}
-
-.column_with_spacing {
+  cursor: pointer;
   display: flex;
-  flex-direction: column;
-  gap: 2em;
-  margin-top: 2em;
-}
-
-.bordered_container {
-  border-right: 1px solid var(--ui-widget-border-color);
-  border-bottom: 1px solid var(--ui-widget-border-color);
-  border-left: 1px solid var(--ui-widget-border-color);
-  border-radius: 4px;
-  box-shadow: var(--ui-widget-dropdown-shadow);
-  padding: 1em;
-  flex: 1;
-  min-height: 0;
+  align-items: center;
+  justify-content: center;
+  z-index: 1;
+  /* background: rgba(0, 0, 255, 0.1); /* temporary debug color */
 }
 
-.logo {
-  transform: scale(0.2);
-  transform-origin: top left;
-  background-color: black;
-  padding-left: 1em;
-  height: 40px;
+/* Remove any potential conflicting styles */
+.expand-button:hover {
+  background-color: transparent;
 }
 
-.header-container {
-  display: flex;
-  background-color: black;
-  padding: 1em;
-  height: 4vh;
-  overflow: hidden;
+.expand-icon {
+  width: 12px;
+  height: 12px;
+  color: var(--ui-widget-secondary-text);
+  opacity: 0.6;
 }
 
-footer {
-  margin-top: auto;
-  padding: 1em;
-  text-align: center;
-  background-color: #f8f8f8;
+.expand-button:hover .expand-icon {
+  opacity: 1;
 }
 
diff --git a/python/examples/marimo/numerous/app.py b/python/examples/marimo/numerous/app.py
index fa7572d..228bb8a 100644
--- a/python/examples/marimo/numerous/app.py
+++ b/python/examples/marimo/numerous/app.py
@@ -113,7 +113,8 @@ def __():
 
 
 @app.cell
-def __():
+def __(aw, wi):
+    aw(wi.Task())
     return