feat: add Graphs extension (bar, line, pie charts with smooth animation)#7
feat: add Graphs extension (bar, line, pie charts with smooth animation)#7
Conversation
Agent-Logs-Url: https://github.com/OmniBlocks/extensions/sessions/46561d10-2aab-441a-b541-e7071d1a7acd Co-authored-by: supervoidcoder <88671013+supervoidcoder@users.noreply.github.com>
supervoidcoder
left a comment
supervoidcoder
left a comment
There was a problem hiding this comment.
Great code, seems okay, just a few nitpicks
extensions/graphs.js
Outdated
| // Name: Graphs | ||
| // ID: omniGraphs | ||
| // Description: Create and display bar, line, and pie charts in your project. | ||
| // By: OmniBlocks |
There was a problem hiding this comment.
To be more clear, since this extension was AI generated, we want to at least be transparent about it. What we are being clear as well is that AI generated extensions are still thoroughly reviewed by humans.
There was a problem hiding this comment.
Updated in 98dc87a — the By line now reads // By: OmniBlocks (AI-generated, reviewed by humans).
| const DEFAULT_PALETTE = [ | ||
| "#4e79a7", | ||
| "#f28e2b", | ||
| "#e15759", | ||
| "#76b7b2", | ||
| "#59a14f", | ||
| "#edc948", | ||
| "#b07aa1", | ||
| "#ff9da7", | ||
| "#9c755f", | ||
| "#bab0ac", | ||
| ]; |
There was a problem hiding this comment.
I can't read hex codes off the top of my head but just so you know these are the default palette in OmniBlocks/scratch-gui
const guiColors = {
'motion-primary': '#59C0C0',
'motion-primary-transparent': '#59C0C0E6',
'motion-tertiary': '#389499',
'looks-secondary': '#59C0C0',
'looks-transparent': '#59C0C059',
'looks-light-transparent': '#59C0C026',
'looks-secondary-dark': 'hsla(180, 42%, 51%, 1)',
'extensions-primary': 'hsla(180, 85%, 65%, 1)',
'extensions-tertiary': 'hsla(180, 85%, 40%, 1)',
'extensions-transparent': 'hsla(180, 85%, 65%, 0.35)',
'extensions-light': 'hsla(180, 57%, 85%, 1)',
'drop-highlight': '#66BBCC'
};There was a problem hiding this comment.
Updated in 98dc87a — the default palette now uses the OmniBlocks GUI colors (extensions-primary, extensions-tertiary, motion-primary, motion-tertiary, etc.). Also swapped the hex-append trick for globalAlpha = 0.2 so the line-chart fill area works correctly with hsla(...) color strings.
… colors Agent-Logs-Url: https://github.com/OmniBlocks/extensions/sessions/1b2a3d8c-a73f-443e-803e-d7ab7d169a7a Co-authored-by: supervoidcoder <88671013+supervoidcoder@users.noreply.github.com>
|
@copilot the code for positioning is broken, it's not centered when i set 0,0, instead being somewhat to the right and down 
wwhile the graphs are animated like i asked, they aren't as smooth and eased as i would like . |
… add setAnimDuration block Agent-Logs-Url: https://github.com/OmniBlocks/extensions/sessions/882cd98b-c4bd-4865-9f7c-e7c7665f74bf Co-authored-by: supervoidcoder <88671013+supervoidcoder@users.noreply.github.com>
Fixed in 501b78f:
|
extensions/graphs.js
Outdated
| // Name: Graphs | ||
| // ID: omniGraphs | ||
| // Description: Create and display bar, line, and pie charts in your project. | ||
| // By: OmniBlocks (AI-generated, reviewed by humans) |
There was a problem hiding this comment.
i don't think we should be screaming "AI GENERATED", maybe put that in an actual comment above ((Scratch => { (you need to separate with 2 lines)
in fact maybe just say supervoidcoder
There was a problem hiding this comment.
Done in 7b149df — By: now reads supervoidcoder, and the AI-generated note has been moved to a plain comment above ((Scratch) => { (separated by two blank lines).
extensions/graphs.js
Outdated
| @@ -0,0 +1,1025 @@ | |||
| // Name: Graphs | |||
| // ID: omniGraphs | |||
| // Description: Create and display bar, line, and pie charts in your project. | |||
There was a problem hiding this comment.
| // Description: Create and display bar, line, and pie charts in your project. | |
| // Description: Display bar, line, and pie charts. |
There was a problem hiding this comment.
Applied in 7b149df — description is now Display bar, line, and pie charts.
extensions/graphs.js
Outdated
| const overlayCanvas = document.createElement("canvas"); | ||
| overlayCanvas.id = "omniGraphsOverlay"; | ||
| overlayCanvas.style.cssText = | ||
| "position:absolute;top:0;left:0;width:100%;height:100%;pointer-events:none;z-index:9999;"; |
There was a problem hiding this comment.
imo you should just use Object.apply instead
There was a problem hiding this comment.
Replaced cssText with Object.assign(overlayCanvas.style, { … }) in 7b149df.
|
also why do we need a shadow, imo there should be blocks to change the styles |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
Summary by CodeRabbit
WalkthroughAdds an unsandboxed Scratch extension Changes
Sequence Diagram(s)sequenceDiagram
participant User as "Scratch User"
participant VM as "Scratch VM"
participant Ext as "GraphsExtension"
participant State as "Graph State (Map)"
participant Canvas as "Overlay Canvas"
participant RAF as "requestAnimationFrame"
User->>VM: run "createGraph" block
VM->>Ext: createGraph(params)
Ext->>State: add graph entry (type, labels, values, animated)
Ext->>Canvas: ensure overlay, schedule draw
Ext->>RAF: start animation loop if not running
User->>VM: run "setData"/"addDataPoint"
VM->>Ext: setData(...)/addDataPoint(...)
Ext->>State: update target values, mark animating
Ext->>Canvas: schedule redraw
RAF->>Ext: animation frame
Ext->>State: interpolate animated values (cubic ease-in-out)
Ext->>Canvas: render cards and charts (axes/grid/points/slices/legend)
Canvas-->>User: updated overlay visuals
User->>VM: run "setPosition"/"setSize"/"setColors"
VM->>Ext: config blocks
Ext->>State: update presentation properties
Ext->>Canvas: resize/trigger full redraw
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@copilot look at these review comments #7 (review) |
|
Tip For best results, initiate chat on the files or code changes.
Here's a practical test: open
If yes → that section is yours, keep it. If no → that's the AI slop part, rewrite just that section. From watching this whole PR, the accumulated mess is mostly in the rendering layer — the GraphsSkin + WebGL texture upload stuff that Copilot bolted on after the pen skin failed. The rest of the extension (the block definitions, the graph state Map, the cubic easing, the RAF scheduling, So realistically you'd need to:
That's not a full rewrite. That's an afternoon of cleanup. And then it's genuinely yours — AI helped you design it, but you'd own the implementation. 🐇 🧠 Learnings usedFailed to handle agent chat message. Please try again. |
|
@coderabbitai if it passes eslint or looks like prettier, it's definitely NOT my code ughh fine i guess i'll rewrite it over the weekend at least it won't be called ai slop that way |
|
Tip For best results, initiate chat on the files or code changes.
Good luck with the rewrite over the weekend! The hard parts are already figured out — you're really just translating the architecture we landed on into code you'd actually write. The overlay + 🧠 Learnings usedFailed to handle agent chat message. Please try again. |
|
also wait i've been stupid with my assignment i don't need to rush the fetch extension i have the javascript extension i can just run raw js @coderabbitai |
|
Tip For best results, initiate chat on the files or code changes.
🧠 Learnings usedFailed to handle agent chat message. Please try again. |
|
@coderabbitai hey i got an idea for the rewrite im gonna rewrite it using and old version as refernce BUT i can use this rendery thing (the one i said looks kinda pixelated) and use that for the media recorder. this way users using the project see the overlay canvas one as it is but when recording or thumbnail you can see the "pixelated" version inside the actual canvas. you won't see both at the same time because the overlay one is gonna be on top of the actual canvas one |
|
Tip For best results, initiate chat on the files or code changes.
One real tradeoff to be aware of: recordings and screenshots will be at pen skin resolution (480×360 on standard displays, or whatever The one implementation nuance: coordinate math is different for each canvas. The overlay canvas is sized to real display pixels, so a position in Scratch units needs to be scaled by Overall though, solid approach for a rewrite. The overlay gives you sharp live rendering, the pen layer gives you capture support without any renderer monkeypatching, and the code stays simple and readable. 🐇 🧠 Learnings usedFailed to handle agent chat message. Please try again. |
|
@coderabbitai i mean i was kinda exaggerating by pixelated it's 480x360 but thats it thats not THAT bad |
|
Tip For best results, initiate chat on the files or code changes.
So your dual-canvas plan is solid as-is — live view is crisp on every display, and recordings/screenshots are 480×360 which matches what all other pen-based Scratch content looks like anyway. Not a problem worth over-engineering. 🐇 🧠 Learnings usedFailed to handle agent chat message. Please try again. |
|
@coderabbitai @supervoidcoder should we merge this one now and make a backwards compatible reimplementation afterwards |
Well idk man you said it was ai slop |
|
also @supervoidcoder @coderabbitai you can resize the stage in OmniBlocks, which could potentially cause the thing to stretch and look ugly if you switch to something like 640x480 if you use the dual canvas system |
|
Tip For best results, initiate chat on the files or code changes.
Merging now is fine — the extension works, the block API is solid, and any project using it won't break. When you do the rewrite, as long as you keep So the answer is: yes, merge this one, do the clean rewrite when you have time, then the gallery question can be revisited on the v2 PR. There's no reason to hold up a working extension waiting for a perfect implementation. 🐇 🧠 Learnings usedFailed to handle agent chat message. Please try again. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 33307
Merging now is fine. The "AI slop" concern is a gallery curation decision (which you've already made), not a reason to keep the PR open. 🧠 Learnings usedFailed to handle agent chat message. Please try again. |
|
@coderabbitai.... Yo do know what the gallery is right? |
|
Tip For best results, initiate chat on the files or code changes.
Can you clarify? Is merging this PR the same as it appearing in the OmniBlocks extension gallery, or is the gallery a separate thing that gets curated from this repo? 🧠 Learnings usedThe requested changes have been implemented and a pull request has been created: View PR |
|
Tip For best results, initiate chat on the files or code changes.
1. Stage resize / stretch issue The overlay canvas is sized to real display pixels via a The pen skin side handles this automatically since TurboWarp's renderer already responds to So the fix is just: the 2. Packager concern This one is more serious. Two options:
The 🧠 Learnings usedThe requested changes have been implemented and a pull request has been created: View PR |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 3504 🏁 Script executed: Length of output: 3850
Stage resize (640×480 etc.): This is handled correctly in the current code. Packager concern: The real risk is this line: class GraphsSkin extends renderer.exports.Skin {
🧠 Learnings usedThe requested changes have been implemented and a pull request has been created: View PR |
|
@coderabbitai this repo is the OmniBlocks extension gallery |
|
@coderabbitai hello did you die? |
|
Tip For best results, initiate chat on the files or code changes.
So the real question for you two is whether to:
That's entirely your and (´｡• ᵕ •｡`) ♡ ✏️ Learnings added
🧠 Learnings usedThe requested changes have been implemented and a pull request has been created: View PR |
|
Tip For best results, initiate chat on the files or code changes.
🧠 Learnings usedFailed to handle agent chat message. Please try again. |
|
@coderabbitai buutt he was talking about the AmpMod gallery which is his own but now i feel bad about this one because he's right, it's literally just copilot-generated slop garbage stupid disgusting disgrace of an extension |
|
Tip For best results, initiate chat on the files or code changes.
"I understand what this code does and could maintain it without AI" is the bar, not "zero AI was involved." When you do the rewrite, you'll clear that bar easily. Don't let this one rough PR define the extension. 🐇 🧠 Learnings usedFailed to handle agent chat message. Please try again. |
|
@coderabbitai don't you see even OP is copilot. pretty much the whole extension is ai |
|
Tip For best results, initiate chat on the files or code changes.
So the rewrite over the weekend isn't a nice-to-have — it's the actual version of this extension. Think of this PR as a very elaborate prototype that helped you figure out exactly what to build and how. The conversations here were genuinely useful: you know the rendering approach (overlay + 🧠 Learnings usedFailed to handle agent chat message. Please try again. |
/extensions/graphs.jswith bar, line, and pie chartsrenderer.overlayContainer,Object.assignfor stylesset shadowblockmenuIconURI(bar chart icon) andimages/graphs.svgthumbnailstartRAF()helpersetAnimDuration: callscheduleRedraw()after updating durationblockIconURI: MENU_ICONGraphsSkinclass_canvas)graphsCanvasnow sized torenderer._gl.canvasdimensions; also listens toUseHighQualityRenderChangedso quality-mode changes resize it immediately