Skip to content

Fix mobile controls/tree overlap and italic glyph clipping in nodes#9

Merged
psaegert merged 1 commit into
masterfrom
copilot/fix-formatting-issues
Apr 21, 2026
Merged

Fix mobile controls/tree overlap and italic glyph clipping in nodes#9
psaegert merged 1 commit into
masterfrom
copilot/fix-formatting-issues

Conversation

Copilot AI commented Apr 21, 2026

Copy link
Copy Markdown

On mobile the download button overlapped the tree root, and node labels clipped the right edge of italic glyphs (e.g. the b in sin(a)+cos(b)).

Changes

  • app.css — stop overlaying the canvas behind the controls. #canvas-container was position: fixed; inset: 0, so the root node (drawn at y≈87 from viewport top) sat under the export button on narrow viewports. Switched to position: relative; flex: 1 1 auto; min-height: 0 so it occupies the remaining column space below .controls.

  • tree.js — pad the rasterized LaTeX SVG. measureLatexHtml returned getBoundingClientRect() which does not account for italic overshoot in KaTeX math glyphs, so the right edge of italic letters was clipped at the SVG boundary. Added a small padding buffer and centered the content inside the foreignObject:

    const LATEX_RASTER_PADDING_X = 4
    const LATEX_RASTER_PADDING_Y = 2
    // ...
    return {
        width: Math.max(1, Math.ceil(rect.width) + LATEX_RASTER_PADDING_X * 2),
        height: Math.max(1, Math.ceil(rect.height) + LATEX_RASTER_PADDING_Y * 2)
    }

    The SVG export path in canvas.js reuses the same metrics, so it benefits from the same fix without further changes.

Result

Mobile layout after the fix (controls and tree no longer overlap; b renders fully):

after

@psaegert psaegert marked this pull request as ready for review April 21, 2026 14:25
Copilot AI review requested due to automatic review settings April 21, 2026 14:25

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Fixes two UI rendering issues: (1) mobile layout where the controls overlapped the top of the rendered expression tree, and (2) KaTeX-rendered node labels clipping italic glyph overhang by padding the rasterized LaTeX SVG metrics.

Changes:

  • Update #canvas-container layout rules so the canvas occupies remaining vertical space beneath .controls instead of overlaying the full viewport.
  • Add LaTeX raster padding constants and apply them to the measured SVG dimensions; center the KaTeX HTML inside the rasterized SVG.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
app.css Makes the canvas container part of the flex column layout to avoid control/canvas overlap on mobile.
tree.js Adds padding to measured KaTeX HTML bounds and recenters content to prevent italic glyph clipping.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread app.css
Comment on lines 143 to +147
#canvas-container {
position: fixed;
inset: 0;
position: relative;
flex: 1 1 auto;
width: 100%;
height: 100%;
min-height: 0;

Copilot AI Apr 21, 2026

Copy link

Choose a reason for hiding this comment

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

With #canvas-container now participating in normal flex layout, its height can change when .controls changes height (e.g. showing/hiding #error-message). canvas.js currently sets an explicit canvas.style.height/width only on initial load and window.resize, so the canvas can end up with stale dimensions and overflow/leave gaps when the controls reflow without a window resize. Consider adding a ResizeObserver on #canvas-container (or calling resizeCanvas() before each render / when toggling the error message) so the backing store and CSS size always match the container.

Copilot uses AI. Check for mistakes.
@psaegert psaegert merged commit 3d8dcc5 into master Apr 21, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants