feat(gnoweb): consolidate built-in playground UI#5565
feat(gnoweb): consolidate built-in playground UI#5565jeronimoalbi wants to merge 24 commits intognolang:playground2from
Conversation
Also add a bit of padding to the tab bar so it looks better.
🛠 PR Checks SummaryAll Automated Checks passed. ✅ Manual Checks (for Reviewers):
Read More🤖 This bot helps streamline PR reviews by verifying automated checks and providing guidance for contributors and reviewers. ✅ Automated Checks (for Contributors):No automated checks match this pull request. ☑️ Contributor Actions:
☑️ Reviewer Actions:
📚 Resources:Debug
|
It's making CI fail
|
@jeronimoalbi Thank you for taking care of this. When you finish, can I jump into your PR to make some adjustments? |
@alexiscolin of course 👍 |
If the ned file name already exists switch to the tab for that file.
Uses the same package path used in `gnokey` addpkg output example.
A ToC could be added here to easily jump to the different sections.
@alexiscolin let me know if you think this PR should cover other aspects. The focus was on making the code in Feel free to push and improve on this PR 👍 |
|
Something that is not quite clear so far is how to deal with the top menu, specially when the Playground is displayed. Right now Playground has no menu because the items didn't make sense within that context. Also we should add a link to the Playground view somewhere. |
That's where I would like to make some improvements. This view is not optimal right now in term of UX 👍 I'm also gonna merge some views to it easier to manipulate. |
alexiscolin
left a comment
There was a problem hiding this comment.
Thanks for pushing this forward Jerónimo. The visual direction is right: dark mode, the header unification across views, the tab typography all land better.
A few inline notes below on small regressions I spotted in the diff. Nothing unfixable.
Three things that aren't in this diff but feel worth raising before we call M0 done, per #5549. Happy to split them as separate PRs on playground2 if you prefer, just want them on the radar:
-
Nav. The default dev links case in
layout_header.gostill returns 6 tabs (Content / Source / Actions / Eval / Fork / Run). I know that came from #5421 not from this PR, but since M0 listsnavas a dimension and M1/M2 will absorb Fork and Eval into Source and Actions, does it make sense to drop them from the header already? URLs stay reachable for deep links, we just don't train users on a 6-tab topology we'll undo. We can keep it in mind for the following PRs. -
Controllers alignment.
controller-playground.ts,controller-eval.ts, andcontroller-run.tsare factory functions, while every other controller in the codebase extendsBaseController. M0 lists this ascontroller alignment, and M1 will need cross-controller dispatch (Source → Playground) which the factory pattern doesn't expose. You're already editing these files here, so the delta would be small. Do in this PR or next one onplayground2? -
Dead code after the
renderRunContentcollapse? Not in the diff butview_run.go:27still wraps data inrunViewParams.Articlepointing torenderRunContent, a template that no longer exists right? Harmless today (never rendered), but a trap if anyone re-addslayout/articleto realign with the other views.
| <div class="c-stack" data-eval-target="history-list"> | ||
| </div> | ||
| <h2 class="b-content-h2">History</h2> | ||
| <div class="b-eval-history-list" data-eval-target="history-list" /> |
There was a problem hiding this comment.
Nit: <div ... /> self-closing syntax isn't valid HTML5 for non-void elements (the / is ignored by the parser). It renders fine here because the </div> on the next line balances it, but the syntax is confusing for readers used to JSX/XHTML.
| <div class="b-eval-result"> | ||
| <div class="b-eval-result-header"> | ||
| <span>Result</span> | ||
| <h3 class="b-eval-result-title">Result</h3> |
There was a problem hiding this comment.
NIT: "Result" went from <span> to <h3>, so DOM order now reads h1 → h3 Result → h2 Quick Call → h2 History, which is backwards for screen readers.
WDYT about keeping it as a span (or a panel-title class if we factor the shared panel primitive later)?
| <div class="b-playground-output"> | ||
| <div class="b-playground-output-header"> | ||
| <span class="b-playground-output-title">Output</span> | ||
| <h3 class="b-playground-output-title">Output</h3> |
There was a problem hiding this comment.
Same thought as on eval: "Output" feels more like a pane label than a section heading. Span rather than <h3>?
| padding: var(--g-space-1-5); | ||
| border: 1px solid var(--s-color-border-default); | ||
| border-radius: var(--g-radius-1); | ||
| border: 1px solid var(--s-color-border-defaul); |
There was a problem hiding this comment.
| border: 1px solid var(--s-color-border-defaul); | |
| border: 1px solid var(--s-color-border-default); |
Co-authored-by: Alexis Colin <alexis@jaunebleu.co>
I would address these in follow-up PRs if you agree, to make it easier to review. We could link back to your comment from the PRs. |








Addresses milestone 0 (M0) from #5549