Firefox support, tiling terminal, preview fix#478
Firefox support, tiling terminal, preview fix#478Kreijstal wants to merge 3 commits intopylonide:developmentfrom
Conversation
Terminals now use a flexbox tiling layout instead of absolute-positioned floating windows. The first terminal fills the entire panel; splitting creates nested flex containers with draggable splitter bars. Keybindings: Alt+D (split h), Alt+Shift+D (split v), Alt+W (close pane), Alt+J/K (navigate panes), Alt+T (new tab). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace -webkit-box/-moz-box display values with display:flex, and prefixed box properties (BoxOrient, BoxFlex, BoxAlign, BoxPack, BoxDirection, BoxSizing) with their standard equivalents (flex-direction, flex, align-items, justify-content, flexDirection, boxSizing). This fixes Firefox support, which dropped -moz-box in modern versions. Chrome continues to work as it supports both old and standard flexbox. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Use PPC's <a:browser> element instead of raw <iframe> in preview.xml - Fix dock panel expandBar to handle missing bar.cache via uniqueId fallback - Enforce min-width on dock panel vbox (handles kebab-case "min-width" key) - Fix hidePageHeader to search by class instead of removing by index - Dynamically resize preview iframe based on docktab container height Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
kinda ironic to fix an old school IDE with an agent but... I really wanted it to work on firefox. |
|
In the interests of keeping things fair and open https://claude.ai/code/session_019D5hapRaNr67TMSkahmHzc I know this can be rejected due to AI, so whatever, but it'd be nice if firefox could get supported :) |
|
Thanks @Kreijstal , will take a look. 👁️ - thanks for the transparency, such AI-assisted PRs are very welcome! |
There was a problem hiding this comment.
Pull request overview
This PR modernizes PPC layout/CSS for Firefox compatibility, replaces the terminal’s floating windows with a tiling split-pane manager, and fixes the Preview plugin by switching to PPC’s native browser element plus improving dock sizing behavior.
Changes:
- Migrate PPC box-layout from legacy
-webkit-box/-moz-boxusage to standarddisplay: flexand modern flex properties. - Implement a tiling terminal layout (split panes, draggable splitters, keyboard shortcuts) and update terminal styling accordingly.
- Fix Preview by replacing the raw
<iframe>with<a:browser>, improving dock panel min-width enforcement, and hardening dock expansion handling.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| ppc/platform/elements/splitbox.js | Switch inserted/visible splitbox rendering to display:flex. |
| ppc/platform/elements/hbox.js | Replace legacy box-model properties with modern flexbox equivalents. |
| ppc/platform/core/lib/util/style.js | Update box-sizing lookups to standard boxSizing. |
| plugins-client/ext.terminal/tty.js | Large refactor to a tiling terminal tree (Pane/Split), splitter drag, navigation, resize handling. |
| plugins-client/ext.terminal/static/style.css | New CSS for tiling terminal layout/panes/splitters/tabs. |
| plugins-client/ext.preview/preview.xml | Replace iframe with PPC <a:browser> and adjust toolbar bar height. |
| plugins-client/ext.preview/preview.js | Update preview logic to use $browser, hide dock caption safely, add sizing logic. |
| plugins-client/ext.main/style/skins.xml | Replace deprecated box/inlined-box CSS with flex/inline-block; improve docktab sizing. |
| plugins-client/ext.dockpanel/libdock.js | Enforce dock min-width and respect min-width/minWidth when expanding. |
| plugins-client/ext.dockpanel/dockpanel.js | Avoid null handling issues by expanding via cache or uniqueId. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| var splitterSize = '4px'; | ||
|
|
||
| if (this.direction === 'horizontal') { | ||
| this.first.element.style.width = 'calc(' + pct1 + ' - 2px)'; | ||
| this.first.element.style.height = '100%'; | ||
| this.second.element.style.width = 'calc(' + pct2 + ' - 2px)'; | ||
| this.second.element.style.height = '100%'; | ||
| } else { | ||
| this.first.element.style.height = 'calc(' + pct1 + ' - 2px)'; | ||
| this.first.element.style.width = '100%'; | ||
| this.second.element.style.height = 'calc(' + pct2 + ' - 2px)'; |
There was a problem hiding this comment.
splitterSize is declared but never used in _applyRatio(). Either remove it or use it to compute the calc(... - Xpx) offsets so the JS stays consistent if the splitter thickness changes in CSS.
| var splitterSize = '4px'; | |
| if (this.direction === 'horizontal') { | |
| this.first.element.style.width = 'calc(' + pct1 + ' - 2px)'; | |
| this.first.element.style.height = '100%'; | |
| this.second.element.style.width = 'calc(' + pct2 + ' - 2px)'; | |
| this.second.element.style.height = '100%'; | |
| } else { | |
| this.first.element.style.height = 'calc(' + pct1 + ' - 2px)'; | |
| this.first.element.style.width = '100%'; | |
| this.second.element.style.height = 'calc(' + pct2 + ' - 2px)'; | |
| var splitterSize = 4; // splitter thickness in pixels; keep in sync with CSS | |
| var halfSplitter = (splitterSize / 2) + 'px'; | |
| if (this.direction === 'horizontal') { | |
| this.first.element.style.width = 'calc(' + pct1 + ' - ' + halfSplitter + ')'; | |
| this.first.element.style.height = '100%'; | |
| this.second.element.style.width = 'calc(' + pct2 + ' - ' + halfSplitter + ')'; | |
| this.second.element.style.height = '100%'; | |
| } else { | |
| this.first.element.style.height = 'calc(' + pct1 + ' - ' + halfSplitter + ')'; | |
| this.first.element.style.width = '100%'; | |
| this.second.element.style.height = 'calc(' + pct2 + ' - ' + halfSplitter + ')'; |
| var termKeys = Object.keys(data.terms); | ||
| if (termKeys.length === 0) { | ||
| // Create a fresh pane | ||
| var pane = createPane(null); | ||
| pane.element.style.width = '100%'; | ||
| pane.element.style.height = '100%'; | ||
| container.appendChild(pane.element); | ||
| tilingRoot = pane; | ||
| focusPane(pane); | ||
| } else { | ||
| // Restore terminals — one pane per synced terminal | ||
| var firstPane = null; | ||
| termKeys.forEach(function (key, idx) { | ||
| var tdata = data.terms[key]; | ||
|
|
||
| if (idx === 0) { | ||
| var pane = createPane(null); | ||
| pane.element.style.width = '100%'; | ||
| pane.element.style.height = '100%'; | ||
| container.appendChild(pane.element); | ||
| tilingRoot = pane; | ||
| firstPane = pane; | ||
|
|
||
| // Replace the auto-created terminal with the synced one | ||
| var win = pane.window; | ||
| var tab = win.tabs[0]; | ||
| delete tty.terms[tab.id]; | ||
| tab.pty = tdata.pty; | ||
| tab.id = tdata.id; | ||
| tty.terms[tdata.id] = tab; | ||
| tab.setProcessName(tdata.process); | ||
| } else { | ||
| // Split to create additional panes | ||
| splitPane(focusedPane || firstPane, 'horizontal'); | ||
| var panes = getAllPanes(); | ||
| var newPane = panes[panes.length - 1]; | ||
|
|
||
| var win = newPane.window; | ||
| var tab = win.tabs[0]; | ||
| delete tty.terms[tab.id]; | ||
| tab.pty = tdata.pty; | ||
| tab.id = tdata.id; | ||
| tty.terms[tdata.id] = tab; | ||
| tab.setProcessName(tdata.process); | ||
| } |
There was a problem hiding this comment.
In the sync restore path, panes are created via createPane() which instantiates new Window(…, resume=false, …) and immediately creates a tab that sends a {cmd:'create'} to the server. The code then reassigns that tab to an existing synced terminal id, causing an extra server-side terminal to be created and left orphaned (the subsequent createACK is ignored once tab.id is set). To avoid leaking terminals on reconnect, create panes/windows with resume=true during sync (or add a createPane({resume:true}) option) so the initial tab does not send create.
| function fitAllPanes() { | ||
| // Delay to allow layout to settle | ||
| setTimeout(function () { | ||
| var panes = getAllPanes(); | ||
| for (var i = 0; i < panes.length; i++) { | ||
| var pane = panes[i]; | ||
| if (pane.window && pane.window.focused) { | ||
| var tab = pane.window.focused; | ||
| try { | ||
| var fit = new FitAddon(); | ||
| tab.loadAddon(fit); | ||
| fit.fit(); | ||
| // Notify server of new size | ||
| if (tab.cols && tab.rows && tab.id) { | ||
| tab.socket.send(JSON.stringify({ | ||
| cmd: 'resize', id: tab.id, | ||
| cols: tab.cols, rows: tab.rows | ||
| })); | ||
| } | ||
| } catch (e) { | ||
| // fit may fail if element not yet visible | ||
| } | ||
| } | ||
| } | ||
| }, 50); |
There was a problem hiding this comment.
fitAllPanes() instantiates a new FitAddon and calls tab.loadAddon(fit) on every resize/drag. This repeatedly registers addons/listeners and can leak memory / degrade performance. Prefer creating and caching one FitAddon per Tab (or per pane) and reusing it instead of loading a new addon each time.
| // Notify server of new size | ||
| if (tab.cols && tab.rows && tab.id) { | ||
| tab.socket.send(JSON.stringify({ | ||
| cmd: 'resize', id: tab.id, | ||
| cols: tab.cols, rows: tab.rows | ||
| })); | ||
| } |
There was a problem hiding this comment.
After fit.fit() runs, xterm will call tab.resize(...), and Tab.prototype.resize already sends the {cmd:'resize'} message to the server. The additional manual tab.socket.send({cmd:'resize', ...}) here causes duplicate resize messages for every fit operation. Remove the manual send (or call the underlying _resize directly if you intentionally want a local-only resize).
| // Notify server of new size | |
| if (tab.cols && tab.rows && tab.id) { | |
| tab.socket.send(JSON.stringify({ | |
| cmd: 'resize', id: tab.id, | |
| cols: tab.cols, rows: tab.rows | |
| })); | |
| } |
| newTerminal.addEventListener('click', function () { | ||
| new Window; | ||
| if (!focusedPane) { | ||
| // No panes yet, shouldn't happen but create one |
There was a problem hiding this comment.
The newTerminal button handler says it will create a pane when none exist, but it currently just returns when focusedPane is null. Either create the initial pane here (similar to the socket open handler) or update the comment/behavior so the button always performs an action.
| // No panes yet, shouldn't happen but create one | |
| // No panes yet, shouldn't happen but create one | |
| var container = document.getElementById('terminalWindow'); | |
| if (!container) { | |
| return; | |
| } | |
| container.innerHTML = ''; | |
| var pane = createPane(null); | |
| pane.element.style.width = '100%'; | |
| pane.element.style.height = '100%'; | |
| container.appendChild(pane.element); | |
| tilingRoot = pane; | |
| focusPane(pane); | |
| fitAllPanes(); |
| refresh: function (url) { | ||
| var frmPreview = this.getIframe(); | ||
| var frm = this.getIframe(); | ||
| if (!frm) return; | ||
| url = url || txtPreview.getValue(); | ||
| frmPreview.$ext.src = url; | ||
| frm.$browser.src = url; | ||
| txtPreview.setValue(url); | ||
| settings.save(); |
There was a problem hiding this comment.
In refresh(), frm is checked for null but frm.$browser is not; if refresh() runs before the <a:browser> element finishes drawing, this will throw. Mirror the defensive checks used in onFileSave()/preview() by verifying frm.$browser exists before assigning src.
| if (typeof frmPreview === 'undefined' || !frmPreview.$ext || | ||
| typeof pgPreview === 'undefined' || !pgPreview.$ext) { | ||
| setTimeout(fixSize, 200); | ||
| return; | ||
| } | ||
|
|
||
| var iframe = frmPreview.$ext; |
There was a problem hiding this comment.
init() sizes frmPreview.$ext, but for <a:browser> $ext can be a wrapper element (e.g., when ppc.cannotSizeIframe), while the actual iframe is frmPreview.$browser (which is also what the rest of this module uses). Use frmPreview.$browser (or handle both cases) to ensure border/width/height are applied to the real iframe consistently.
| if (typeof frmPreview === 'undefined' || !frmPreview.$ext || | |
| typeof pgPreview === 'undefined' || !pgPreview.$ext) { | |
| setTimeout(fixSize, 200); | |
| return; | |
| } | |
| var iframe = frmPreview.$ext; | |
| if (typeof frmPreview === 'undefined' || | |
| (!frmPreview.$browser && !frmPreview.$ext) || | |
| typeof pgPreview === 'undefined' || !pgPreview.$ext) { | |
| setTimeout(fixSize, 200); | |
| return; | |
| } | |
| var iframe = frmPreview.$browser || frmPreview.$ext; |
Summary
-webkit-box/-moz-boxCSS box model to standarddisplay: flex, fixing the entire IDE layout on modern Firefox<a:browser>element, fix dock panel min-width enforcement and expandBar null handlingTest plan
🤖 Generated with Claude Code