Viewer improvements #2909
Viewer improvements #2909
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Ignored Deployment
|
|
Apply Sweep Rules to your PR?
|
Resolving merge conflicts: track the progress here.I'm currently resolving the merge conflicts in this PR. I will stack a new PR once I'm done. An error has occurred: 403 {"message": "API rate limit exceeded for 24.144.92.8. (But here's the good news: Authenticated requests get a higher rate limit. Check out the documentation for more details.)", "documentation_url": "https://docs.github.com/rest/overview/resources-in-the-rest-api#rate-limiting"} (tracking ID: f795f5328f) |
Can you add a small down-arrow on the right of this button, to make it more obvious that it's a dropdown? |
| return <MessageAlert heading="Value is not defined" />; | ||
| } | ||
| if (!rootValue.ok) { | ||
| return <SquiggleErrorAlert error={rootValue.value} />; |
There was a problem hiding this comment.
why is this SquiggleErrorAlert, but the others MessageAlert?
There was a problem hiding this comment.
The name is the same as it was before, but I think it's good:
- this component takes
SqError, so it's squiggle-specific - unlike
<MessageAlert>, this component is public, and other components haveSquiggleprefix
|
|
||
| import { SquiggleOutput } from "../../lib/hooks/useSquiggle.js"; | ||
|
|
||
| export const Indicator: FC<{ output: SquiggleOutput; isRunning: boolean }> = ({ |
There was a problem hiding this comment.
Indicator isn't very descriptive. Maybe, RenderingIndicator?
| )} | ||
| > | ||
| <Button size="small"> | ||
| {mode === "variables" ? "Variables" : "Result"} |
There was a problem hiding this comment.
No problem, but note that it won't look identically to Save (no vertical bar). Save button acts differently based on where you click.
|
Runtime errors seem to have lost their blue Stack Traces, from the Storybook: 
|
This is intentional; stack trace lines shouldn't be clickable when there's no editor. |
SquiggleViewerprops; onlySquiggleChartsupportsrootPathOverridenow, whileSquiggleViewertakes the plainSqValueDynamicSquiggleViewertoSquiggleOutputViewerand change the top level navigation to a dropdownSquiggleErrorAlert(no one needs it now but since we exposeSquiggleViewer, we should expose it too)There might be other minor changes here and bugfixes that I've forgot about.