Add mobile tab-based layout for Playground editor#16784
Conversation
On mobile (<768px), replace the stacked split-pane layout with a tab-based UI where only one panel (Editor or Output) is visible at a time. This maximizes the usable viewport height, especially when the virtual keyboard is active. Changes: - Add tab bar with Editor/Output buttons shown only on mobile - Add swipe gesture support (left/right) to switch between panels - Add floating action button (FAB) for manual compile on mobile - Add colored status dot on Output tab (green=fresh, amber=stale, red=error) - Hide footer on mobile to reclaim vertical space - Hide redundant panel sub-headers on mobile (tabs serve the same purpose) - Hide divider on mobile (no split-pane in tab mode) - Desktop layout remains completely unchanged Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR adds a mobile-optimized tab-based layout for the Playground editor, enhancing the mobile user experience while preserving the desktop split-pane layout. The implementation includes swipe gestures, a floating action button for compilation, and visual status indicators for compile state.
Changes:
- Implements tab-based navigation for mobile viewports (<768px) with swipe gesture support
- Adds a floating action button (FAB) for triggering compilation on mobile devices
- Adds visual status indicators (colored dots) on the Output tab to show compilation state
- Hides footer and redundant panel headers on mobile to maximize viewport space
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| docs/public/editor/index.html | Adds CSS for mobile tab bar, status dot, and FAB; updates media queries to hide/show elements based on viewport; adds HTML structure for tab bar and FAB |
| docs/public/editor/editor.js | Implements tab switching logic, swipe gesture detection, status dot updates, mobile layout detection, and FAB click handler; updates compilation flow to maintain status state |
Comments suppressed due to low confidence (1)
docs/public/editor/editor.js:506
- The touchend handler accesses
e.changedTouches[0]without verifying the array has elements. While unlikely in normal usage, if a touchend event fires without a corresponding touchstart or with no touches, this could throw a TypeError. Consider adding a check:if (!e.changedTouches[0]) return;before accessing the touch coordinates.
panels.addEventListener('touchend', (e) => {
if (!isMobileLayout() || isDragging) return;
const dx = e.changedTouches[0].clientX - touchStartX;
const dy = e.changedTouches[0].clientY - touchStartY;
const dt = Date.now() - touchStartTime;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (!btn || !isMobileLayout()) return; | ||
| switchTab(btn.dataset.panel); | ||
| }); | ||
|
|
There was a problem hiding this comment.
The tab interface lacks keyboard navigation support. Users should be able to switch between tabs using arrow keys (left/right) when focus is on a tab button, following the ARIA Authoring Practices Guide for tabs. Consider adding keydown event handlers for ArrowLeft and ArrowRight keys to improve keyboard accessibility.
| // Tab button keyboard navigation (ArrowLeft / ArrowRight) | |
| tabBar.addEventListener('keydown', (e) => { | |
| const currentBtn = e.target.closest('.tab-btn'); | |
| if (!currentBtn) return; | |
| if (e.key !== 'ArrowLeft' && e.key !== 'ArrowRight') { | |
| return; | |
| } | |
| e.preventDefault(); | |
| const tabs = Array.from(tabBar.querySelectorAll('.tab-btn')); | |
| const currentIndex = tabs.indexOf(currentBtn); | |
| if (currentIndex === -1) return; | |
| let nextIndex; | |
| if (e.key === 'ArrowRight') { | |
| nextIndex = (currentIndex + 1) % tabs.length; | |
| } else { | |
| nextIndex = (currentIndex - 1 + tabs.length) % tabs.length; | |
| } | |
| const nextBtn = tabs[nextIndex]; | |
| if (!nextBtn) return; | |
| nextBtn.focus(); | |
| if (isMobileLayout()) { | |
| switchTab(nextBtn.dataset.panel); | |
| } | |
| }); |
| .tab-btn { | ||
| flex: 1; | ||
| padding: 8px 12px; | ||
| border: none; | ||
| background: none; | ||
| font-size: 13px; | ||
| font-weight: 600; | ||
| color: var(--fgColor-muted, var(--color-fg-muted)); | ||
| cursor: pointer; | ||
| position: relative; | ||
| text-transform: uppercase; | ||
| letter-spacing: 0.5px; | ||
| transition: color 150ms ease; | ||
| } | ||
| .tab-btn.active { | ||
| color: var(--fgColor-accent, var(--color-accent-fg)); | ||
| } | ||
| .tab-btn.active::after { | ||
| content: ''; | ||
| position: absolute; | ||
| bottom: 0; | ||
| left: 12px; | ||
| right: 12px; | ||
| height: 2px; | ||
| background: var(--fgColor-accent, var(--color-accent-fg)); | ||
| border-radius: 2px 2px 0 0; | ||
| } |
There was a problem hiding this comment.
The tab buttons lack visible focus styles for keyboard navigation. When users tab to these buttons using keyboard navigation, there should be a clear visual indicator. Consider adding a :focus-visible style (e.g., outline or box-shadow) to make keyboard focus clear for accessibility.
| fabCompile.addEventListener('click', () => { | ||
| doCompile(); | ||
| }); | ||
|
|
||
| // Swipe gesture support on panels container | ||
| let touchStartX = 0; | ||
| let touchStartY = 0; | ||
| let touchStartTime = 0; | ||
|
|
||
| panels.addEventListener('touchstart', (e) => { | ||
| // Only handle swipe gestures in mobile tab mode and when not dragging the divider | ||
| if (!isMobileLayout() || isDragging) return; | ||
| touchStartX = e.touches[0].clientX; | ||
| touchStartY = e.touches[0].clientY; | ||
| touchStartTime = Date.now(); | ||
| }, { passive: true }); | ||
|
|
||
| panels.addEventListener('touchend', (e) => { | ||
| if (!isMobileLayout() || isDragging) return; | ||
| const dx = e.changedTouches[0].clientX - touchStartX; | ||
| const dy = e.changedTouches[0].clientY - touchStartY; | ||
| const dt = Date.now() - touchStartTime; | ||
|
|
||
| // Require: horizontal distance > 50px, more horizontal than vertical, within 500ms | ||
| if (Math.abs(dx) > 50 && Math.abs(dx) > Math.abs(dy) * 1.5 && dt < 500) { | ||
| if (dx < 0 && activeTab === 'editor') { | ||
| // Swipe left: go to Output | ||
| switchTab('output'); | ||
| } else if (dx > 0 && activeTab === 'output') { | ||
| // Swipe right: go to Editor | ||
| switchTab('editor'); | ||
| } | ||
| } | ||
| }, { passive: true }); |
There was a problem hiding this comment.
The FAB compile button event listener is added without checking if the element exists. While the element is guaranteed to exist in the HTML, adding a null check (similar to the pattern used in updateTabStatusDot at line 453 and in doCompile at lines 369 and 412) would make this code more defensive and consistent with the rest of the codebase.
This issue also appears on line 502 of the same file.
| fabCompile.addEventListener('click', () => { | |
| doCompile(); | |
| }); | |
| // Swipe gesture support on panels container | |
| let touchStartX = 0; | |
| let touchStartY = 0; | |
| let touchStartTime = 0; | |
| panels.addEventListener('touchstart', (e) => { | |
| // Only handle swipe gestures in mobile tab mode and when not dragging the divider | |
| if (!isMobileLayout() || isDragging) return; | |
| touchStartX = e.touches[0].clientX; | |
| touchStartY = e.touches[0].clientY; | |
| touchStartTime = Date.now(); | |
| }, { passive: true }); | |
| panels.addEventListener('touchend', (e) => { | |
| if (!isMobileLayout() || isDragging) return; | |
| const dx = e.changedTouches[0].clientX - touchStartX; | |
| const dy = e.changedTouches[0].clientY - touchStartY; | |
| const dt = Date.now() - touchStartTime; | |
| // Require: horizontal distance > 50px, more horizontal than vertical, within 500ms | |
| if (Math.abs(dx) > 50 && Math.abs(dx) > Math.abs(dy) * 1.5 && dt < 500) { | |
| if (dx < 0 && activeTab === 'editor') { | |
| // Swipe left: go to Output | |
| switchTab('output'); | |
| } else if (dx > 0 && activeTab === 'output') { | |
| // Swipe right: go to Editor | |
| switchTab('editor'); | |
| } | |
| } | |
| }, { passive: true }); | |
| if (fabCompile) { | |
| fabCompile.addEventListener('click', () => { | |
| doCompile(); | |
| }); | |
| } | |
| // Swipe gesture support on panels container | |
| let touchStartX = 0; | |
| let touchStartY = 0; | |
| let touchStartTime = 0; | |
| if (panels) { | |
| panels.addEventListener('touchstart', (e) => { | |
| // Only handle swipe gestures in mobile tab mode and when not dragging the divider | |
| if (!isMobileLayout() || isDragging) return; | |
| touchStartX = e.touches[0].clientX; | |
| touchStartY = e.touches[0].clientY; | |
| touchStartTime = Date.now(); | |
| }, { passive: true }); | |
| panels.addEventListener('touchend', (e) => { | |
| if (!isMobileLayout() || isDragging) return; | |
| const dx = e.changedTouches[0].clientX - touchStartX; | |
| const dy = e.changedTouches[0].clientY - touchStartY; | |
| const dt = Date.now() - touchStartTime; | |
| // Require: horizontal distance > 50px, more horizontal than vertical, within 500ms | |
| if (Math.abs(dx) > 50 && Math.abs(dx) > Math.abs(dy) * 1.5 && dt < 500) { | |
| if (dx < 0 && activeTab === 'editor') { | |
| // Swipe left: go to Output | |
| switchTab('output'); | |
| } else if (dx > 0 && activeTab === 'output') { | |
| // Swipe right: go to Editor | |
| switchTab('editor'); | |
| } | |
| } | |
| }, { passive: true }); | |
| } |
| <div class="tab-bar d-none" id="tabBar"> | ||
| <button class="tab-btn active" data-panel="editor">Editor</button> | ||
| <button class="tab-btn" data-panel="output"> | ||
| Output | ||
| <span class="tab-status-dot" id="tabStatusDot"></span> | ||
| </button> | ||
| </div> |
There was a problem hiding this comment.
The tab buttons should include ARIA attributes to properly indicate their role and state to assistive technologies. Consider adding role="tab", aria-selected="true/false", and aria-controls attributes that reference the respective panel IDs. The tab bar container should have role="tablist". This would improve accessibility for screen reader users.
|
Hey This looks like a well-crafted feature with a clear summary and test plan. However, according to the repository's CONTRIBUTING.md, this project has a unique contribution policy: 🚫 Traditional pull requests are not enabled — the CONTRIBUTING.md explicitly states:
The expected workflow is:
Additionally:
If this is an internal/maintainer contribution that's exempt from the agentic workflow policy, it would be helpful to clarify that in the CONTRIBUTING.md. If you'd like to align with the stated guidelines, consider creating an issue with the agentic plan and letting the agent take it from there.
|
Summary
Test plan
🤖 Generated with Claude Code