-
Notifications
You must be signed in to change notification settings - Fork 279
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
docs: Add demo to homepage #882
Conversation
Deploying with Cloudflare Pages
|
Codecov Report
@@ Coverage Diff @@
## main #882 +/- ##
==========================================
+ Coverage 67.51% 67.79% +0.28%
==========================================
Files 62 62
Lines 8206 8120 -86
Branches 1774 1769 -5
==========================================
- Hits 5540 5505 -35
+ Misses 2659 2608 -51
Partials 7 7
Continue to review full report at Codecov.
|
btw I disabled netlify because they greedily tried to bill us just for using the free version - we'll want to switch to cloudflare pages (and nestle panes in the same build since it doesn't support monorepos) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great to me! Everything very clean :P. The syntax highlighting issue (see below) is the most important thing to look into for me.
const stepped = stepUntilConvergence(initState); | ||
this.penroseState = resample(await prepareState(compilerResult.value)); | ||
} else { | ||
console.log(showError(compilerResult.error)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not in scope: should probably do better error handling like rendering errors on the canvas directly.
}; | ||
|
||
renderCanvas = async (state: PenroseState | undefined) => { | ||
componentDidUpdate = async (prevProps: ISimpleProps) => { | ||
if ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A little surprised that prop updates need to be handled this manually. Wondering if we are leveraging the React lifecycle fully.
> | ||
<Listing value={example.sub} width={props.width} height={props.width} /> | ||
<div style={{ width: props.width, height: props.width }}> | ||
<Simple |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice refactoring of Simple
! Maybe this is not "simple" anymore... we should probably rename it to Diagram
or something.
@@ -1,12 +1,12 @@ | |||
export const oneSet = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like there's a way to reference to static files in storybook. Perhaps we can just load files from examples
directly: https://storybook.js.org/docs/react/configure/images-and-assets#serving-static-files-via-storybook-configuration.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great to me! Let's merge it
width: string; | ||
height: string; | ||
}) => { | ||
const env = compileDomain(domain).unsafelyUnwrap(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not in scope: same thing as the diagram component, some error handling would be good.
Description
This PR adds a new
Demo
component which cycles through a fixed collection of trios/variations (changing every 5 seconds), showing the Substance program and animating the diagram's convergence at 60 fps.Implementation strategy and design decisions
We expected that it would be straightforward to embed a React component in the Docusaurus site, but it was not. @wodeni and @maxkrieger put in a lot of effort to get this working, including replacing the build systems for a few of our packages. This also involved an upgrade to (among other dependencies) Docusaurus itself, which required us to update the filenames of some of our images from the tutorials.
Also, as part of this PR, I essentially completely rewrote the
Simple
Penrose diagram component. Now it has more features and also implements its React state management in a somewhat more idiomatic way.Checklist
diagrams/
folderOpen questions
The
Demo
component UI is still pretty barebones:@joshsunshine suggested that perhaps @hsharriman might be able to help improve this UI in the future.
Also, we need some spicier diagrams to actually show in the
Demo
component on the homepage. Perhaps @keenancrane has some up his sleeve?