Overhaul UI to minimal real-time expression tree visualizer with dark mode and synchronous rendering#4
Conversation
…ering Agent-Logs-Url: https://github.com/psaegert/expression-tree-gen/sessions/e008f843-b6be-49f2-867a-0a32bd59bc0a Co-authored-by: psaegert <36567814+psaegert@users.noreply.github.com>
Agent-Logs-Url: https://github.com/psaegert/expression-tree-gen/sessions/e008f843-b6be-49f2-867a-0a32bd59bc0a Co-authored-by: psaegert <36567814+psaegert@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR refactors the app into a minimal, real-time expression tree visualizer: input changes (debounced) trigger synchronous re-rendering, invalid expressions preserve the last valid tree while showing a warning indicator, and a persisted light/dark theme is introduced with theme-aware canvas drawing.
Changes:
- Replaced the button/animation-driven flow with debounced, input-driven rendering and “keep last valid tree” behavior.
- Removed animation/export background modes and simplified canvas rendering to synchronous drawing.
- Rebuilt the UI layout and styling for a minimal centered design with dark-mode support and icon-based controls.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
index.html |
Restructures markup to minimal controls, icon buttons, footer GitHub link, theme toggle, and adds SRI for MathJS. |
app.css |
Replaces legacy styles with a minimal layout + CSS-variable-based light/dark theme tokens. |
canvas.js |
Implements debounced rendering, “last valid tree” behavior, export, resize handling, and persisted theme toggling. |
tree.js |
Removes edge animation, makes rendering synchronous, and reads draw colors from CSS variables. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| canvas.height = container.offsetHeight | ||
| canvas.width = container.offsetWidth | ||
| } | ||
|
|
||
| function clearCanvas() { | ||
| context.clearRect(0, 0, canvas.width, canvas.height) |
There was a problem hiding this comment.
resizeCanvas() sizes the drawing buffer from container.offsetWidth/offsetHeight, but the canvas is styled to width/height: 100% inside a padded container (#canvas-container). This causes the canvas to be scaled by CSS (blurry) and can shift/clip the tree coordinates. Consider sizing from canvas.getBoundingClientRect() (and applying devicePixelRatio) so the backing store matches the rendered canvas size.
| canvas.height = container.offsetHeight | |
| canvas.width = container.offsetWidth | |
| } | |
| function clearCanvas() { | |
| context.clearRect(0, 0, canvas.width, canvas.height) | |
| var rect = canvas.getBoundingClientRect() | |
| var dpr = window.devicePixelRatio || 1 | |
| var width = Math.round(rect.width * dpr) | |
| var height = Math.round(rect.height * dpr) | |
| if (canvas.width !== width || canvas.height !== height) { | |
| canvas.width = width | |
| canvas.height = height | |
| } | |
| context.setTransform(1, 0, 0, 1, 0, 0) | |
| context.scale(dpr, dpr) | |
| } | |
| function clearCanvas() { | |
| var rect = canvas.getBoundingClientRect() | |
| context.clearRect(0, 0, rect.width, rect.height) |
| } | ||
|
|
||
| function setWarningVisible(isVisible) { | ||
| warning.hidden = !isVisible |
There was a problem hiding this comment.
setWarningVisible() only toggles visibility of the warning icon. Since the warning <span> is aria-hidden and the input doesn’t get aria-invalid, screen readers won’t be informed when the expression becomes invalid. Consider toggling input.setAttribute('aria-invalid', ...) and optionally wiring aria-describedby to a non-hidden message region.
| warning.hidden = !isVisible | |
| warning.hidden = !isVisible | |
| input.setAttribute('aria-invalid', isVisible ? 'true' : 'false') | |
| warning.setAttribute('aria-hidden', isVisible ? 'false' : 'true') |
| function getPreferredTheme() { | ||
| var storedTheme = localStorage.getItem(THEME_STORAGE_KEY) | ||
| if (storedTheme === 'light' || storedTheme === 'dark') { | ||
| return storedTheme | ||
| } | ||
| return window.matchMedia && window.matchMedia('(prefers-color-scheme: dark)').matches ? 'dark' : 'light' |
There was a problem hiding this comment.
getPreferredTheme() reads from localStorage unguarded; in some environments (e.g., certain privacy modes / blocked storage) localStorage.getItem can throw and prevent the app from loading. Consider wrapping localStorage access in a try/catch and falling back to prefers-color-scheme when storage is unavailable.
| const styles = getComputedStyle(document.documentElement) | ||
| const radius = this.getRadius(context) | ||
| const childRadius = childNode.getRadius(context) | ||
| context.strokeStyle = 'gray'; | ||
| const edgeColor = styles.getPropertyValue('--edge').trim() || '#9e9e9e' | ||
| context.strokeStyle = edgeColor |
There was a problem hiding this comment.
getComputedStyle(document.documentElement) is executed for every edge draw. For larger trees this adds avoidable overhead. Consider computing the theme colors once per render (e.g., in drawTree or the caller) and passing the resolved edge color down or setting context.strokeStyle once before traversing.
| const styles = getComputedStyle(document.documentElement) | ||
| const radius = this.getRadius(context) | ||
| const nodeFill = styles.getPropertyValue('--node-fill').trim() || '#ffffff' | ||
| const nodeStroke = styles.getPropertyValue('--node-stroke').trim() || '#212121' | ||
| const textColor = styles.getPropertyValue('--fg').trim() || '#212121' |
There was a problem hiding this comment.
getComputedStyle(document.documentElement) is executed for every node draw. For large trees this can become a bottleneck. Consider resolving --node-fill/--node-stroke/--fg once per render and passing them into Node.draw (or caching them on the context) instead of re-querying computed styles per node.
| <button class="icon-btn input-icon" id="clear-tree" aria-label="Clear expression"> | ||
| <svg viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round" aria-hidden="true"> | ||
| <polyline points="3 6 5 6 21 6"></polyline> |
There was a problem hiding this comment.
These icon buttons don’t specify type="button". It’s fine in the current markup (no <form>), but adding type="button" prevents accidental form submission if the controls are ever placed inside a form later.
This PR replaces the button-driven, animation-heavy UI with a minimal real-time expression tree visualizer. Rendering now updates on debounced input, preserves the last valid tree on invalid expressions, and adds a persistent light/dark theme with theme-aware canvas drawing.
Real-time input-driven rendering
Generateflow; added debounced (~150ms) render on#expression-inputchanges.Rendering pipeline simplification (no animation)
drawEdgeAnimated, RAF waypoint logic, animate plumbing).drawTreeis now synchronous; edges are drawn in a single stroke.Control/UI redesign
Export behavior
canvas.toDataURL('image/png')), no white-background mode.Layout + theming
https://github.com/psaegert/expression-tree-gen.localStorage(etg-theme), with first-load fallback toprefers-color-scheme.Security hardening
crossorigin="anonymous"to the MathJS CDN script tag.Warning
Firewall rules blocked me from connecting to one or more addresses (expand for details)
I tried to connect to the following addresses, but was blocked by firewall rules:
cdnjs.cloudflare.com/usr/bin/curl curl -Ls REDACTED(dns block)If you need me to access, download, or install something from one of these locations, you can either:
Original prompt
Goal
Major UX/UI overhaul. Strip the app down to a minimal, centered, real-time expression tree visualizer with a dark-mode toggle.
Scope summary
Files involved
index.html— restructure markupapp.css— rewrite the layout, remove unused styles, add dark-mode variablescanvas.js— real-time input listener with debounce, remove generate/animate logic, wire icon buttons, handle dark modetree.js— remove all animation code (drawEdgeAnimated, the waypoint/requestAnimationFrame logic). Edges drawn synchronously.drawTreebecomes synchronous. Node/edge colors become theme-aware.Detailed requirements
1. Real-time rendering (
canvas.js)inputevent listener to#expression-input.setTimeout/clearTimeoutpattern — no library).infixToPostfix.constructTree→setCoordinates→clearCanvas→drawTree. Hide the warning indicator.displayErrorMessage()SweetAlert popup and the SweetAlert CDN<script>tag fromindex.html.setTimeoutdance).Warning indicator: A small⚠️ emoji (or icon) positioned, say, inside the input field on the right — to the left of the trash icon, only visible when the current input is invalid. A simple absolutely-positioned
<span>overlaying the input, toggled via a CSS class like.invalid, works well.2. Remove animation entirely (
tree.js,canvas.js)drawEdgeAnimatedand all waypoint/requestAnimationFramecode.Node.drawEdge(context, childNode)becomes synchronous: compute the trimmed endpoints (the logic already computed —startX/Y,endX/Ywith radius trimming) and do a singlebeginPath/moveTo/lineTo/stroke. Noresolvecallback, no promise.drawTree(root, context)becomes a synchronous, non-async function. Iterate children and recurse. Draw edges before/after children as appropriate — the current order works.animateparameter and the#animate-toggleelement, its CSS, and all related code.3. Input field with trash-can icon
<input>in a container so icons can be absolutely positioned inside it.currentColor. Reasonable SVG source: the Feathertrash-2icon.#clear-treebutton behavior).4. Save icon (export)
downloadorsaveinline SVG.<select>entirely).5. Always transparent export
#export-bg-modeselect and all branching on'white'vs'transparent'.canvas.toDataURL('image/png')on the current canvas.<a download>element.6. Minimal centered layout (
app.css,index.html).contact-sectionwith the SVG gradient background.#canvas-containerdotted SVG pattern background.<h1>Expression Tree Generator</h1>title..github-linkin the top-right corner.This pull request was created from Copilot chat.