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

fix: improve ipynb loading hitches #132

Merged
merged 5 commits into from
Aug 22, 2024
Merged

fix: improve ipynb loading hitches #132

merged 5 commits into from
Aug 22, 2024

Conversation

ashtonG
Copy link
Contributor

@ashtonG ashtonG commented Aug 21, 2024

this fixes three issues that happen when loading an ipynb file in the code editor:

  1. when an ipynb file was selected and the file content wasn't loaded, the error handler was called.

  2. when a parse error occurred, the error handler was called with incorrect arguments.

  3. when loading an ipynb file, the unparsed file content renders to the page briefly, even when the content is valid.

We also update the loadable usage in the codeeditor to use the loadable class methods and prevent the filename info from shrinking and growing during the file loading process.

this fixes three issues that happen when loading an ipynb file in the
code editor:

1) when an ipynb file was selected and the file content wasn't loaded,
the error handler was called

2) when a parse error occurred, the error handler was called with
incorrect arguments

3) when loading an ipynb file, the unparsed file content renders to the
page briefly even when the content is valid

We also update the loadable usage in the codeeditor to use the loadable class.
Copy link

netlify bot commented Aug 21, 2024

Deploy Preview for hew-ui ready!

Name Link
🔨 Latest commit 92a5e4a
🔍 Latest deploy log https://app.netlify.com/sites/hew-ui/deploys/66c76bd8d43f6500084003c2
😎 Deploy Preview https://deploy-preview-132--hew-ui.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@determined-ci
Copy link
Collaborator

determined-ci commented Aug 21, 2024

Hello! DesignKit diffs for commit 92a5e4a are available for you to view here

@codecov-commenter
Copy link

codecov-commenter commented Aug 21, 2024

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.78%. Comparing base (8bc776f) to head (92a5e4a).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #132   +/-   ##
=======================================
  Coverage   82.78%   82.78%           
=======================================
  Files          49       49           
  Lines        6482     6482           
  Branches      454      454           
=======================================
  Hits         5366     5366           
  Misses       1116     1116           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@EmilyBonar EmilyBonar left a comment

Choose a reason for hiding this comment

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

Looks good, though personally I find the pipes harder to read.

@ashtonG
Copy link
Contributor Author

ashtonG commented Aug 22, 2024

good point -- i think it's useful to represent the result as an Either (as opposed to string | error), but the data-last functions and piping can be hard to follow. i'll add comments explaining what's going on for posterity.

@ashtonG ashtonG merged commit 7ddd2b5 into main Aug 22, 2024
9 checks passed
@ashtonG ashtonG deleted the bug/ET-741/ipynb-fixes branch August 22, 2024 16:54
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.

4 participants