Skip to content
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: Bring back the demo #958

Merged
merged 5 commits into from May 12, 2022
Merged

docs: Bring back the demo #958

merged 5 commits into from May 12, 2022

Conversation

samestep
Copy link
Collaborator

@samestep samestep commented May 11, 2022

Description

This PR reenables the homepage demo which was added in #882 and then removed in #916. I don't remember the exact reasoning for removing the demo before; @cmumatt could you comment on this? If the problem is simply that the text should be tweaked, I can do that in this PR, I just wasn't sure exactly what needed to change.

In any case, now that #904 has made a @penrose/examples package available to @penrose/docs-site, we can simply use the more sophisticated trio we show in the README as of #955. The animation is pretty fun to watch, in my opinion.

Finally, thanks to @wodeni, this PR also adds dark mode support to the demo.

Implementation strategy and design decisions

Docusaurus doesn't seem to correctly handle the spread operator for ES6 iterators. Specifically, given this:

const [...foo] = arr.keys();

it assigns this to foo:

arr.keys().slice(0)

And it translates this:

[...arr.keys()]

into this:

[].concat(arr.keys()

These are very wrong, and they cause things to break when we use @penrose/core or @penrose/components in @penrose/docs-site. To fix this, I replaced this pattern with Array.from throughout our entire codebase.

Checklist

  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new ESLint warnings
  • I have reviewed any generated changes to the diagrams/ folder

@codecov
Copy link

codecov bot commented May 11, 2022

Codecov Report

Merging #958 (f921ebe) into main (199e9c6) will decrease coverage by 0.06%.
The diff coverage is 55.55%.

@@            Coverage Diff             @@
##             main     #958      +/-   ##
==========================================
- Coverage   62.82%   62.75%   -0.07%     
==========================================
  Files          62       62              
  Lines        7857     7889      +32     
  Branches     1821     1842      +21     
==========================================
+ Hits         4936     4951      +15     
- Misses       2797     2814      +17     
  Partials      124      124              
Impacted Files Coverage Δ
packages/core/src/compiler/Domain.ts 86.22% <ø> (ø)
packages/core/src/synthesis/Synthesizer.ts 28.13% <0.00%> (ø)
packages/core/src/analysis/SubstanceAnalysis.ts 75.82% <66.66%> (ø)
packages/core/src/compiler/Substance.ts 92.36% <100.00%> (ø)
packages/core/src/contrib/Minkowski.ts 98.82% <100.00%> (ø)
packages/core/src/synthesis/Search.ts 77.29% <0.00%> (-4.86%) ⬇️
packages/core/src/compiler/Style.ts 78.83% <0.00%> (-0.02%) ⬇️
packages/core/src/types/ast.ts 100.00% <0.00%> (ø)
packages/core/src/engine/Evaluator.ts 32.55% <0.00%> (ø)
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 199e9c6...f921ebe. Read the comment docs.

@cloudflare-pages
Copy link

cloudflare-pages bot commented May 11, 2022

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: f921ebe
Status: ✅  Deploy successful!
Preview URL: https://489a12b3.penrose-panes.pages.dev

View logs

@samestep

This comment was marked as resolved.

@samestep samestep marked this pull request as draft May 11, 2022 20:40
@cmumatt
Copy link
Contributor

cmumatt commented May 11, 2022

@samestep IIRC, the demo was removed only because it wasn't working at that time and we had to get the docs-site published. So, probably just unfortunate timing. I think, generally, everyone was in favor of an animated demo as long as it worked, loaded fast, and looked nice.

@samestep
Copy link
Collaborator Author

@cmumatt OK, makes sense! Thanks for the clarification.

@samestep

This comment was marked as resolved.

@samestep samestep marked this pull request as ready for review May 12, 2022 01:12
Copy link
Member

@wodeni wodeni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great. Thanks again for going through all the pain to find out what went wrong!!

@samestep
Copy link
Collaborator Author

Looks great. Thanks again for going through all the pain to find out what went wrong!!

@wodeni You helped quite a bit too!

@samestep samestep merged commit 54a66cd into main May 12, 2022
@samestep samestep deleted the uncomment-demo branch May 12, 2022 01:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants