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

Initial mesh support #3

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

thangngoc89
Copy link

Hi,

Thanks for making niivue-react! I’ve been using it for a while now, and it’s been awesome. Your diffing method is genius—way better than the way I was reacting (pun intented) to props changes in my old implementation. I ripped it out as soon as I see yours.

Recently, I needed mesh support for my project, so I put together this PR.

Right now, the support is pretty basic but gets the job done:

  • Load/remove mesh
  • Change mesh property (rgba255, visible, ...)
  • Change niivue's instance meshXRay

I’ll add more features as I need them for my project.

Cheers

@jennydaman
Copy link
Collaborator

Amazing! I’ll review later today or tomorrow.

Copy link

codecov bot commented Jan 21, 2025

Codecov Report

Attention: Patch coverage is 97.26027% with 2 lines in your changes missing coverage. Please review.

Project coverage is 79.62%. Comparing base (d7734b8) to head (fe93b1f).
Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
src/NiivueMutator.ts 97.22% 0 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main       #3      +/-   ##
==========================================
- Coverage   81.76%   79.62%   -2.14%     
==========================================
  Files           5        5              
  Lines         784      844      +60     
  Branches      153      154       +1     
==========================================
+ Hits          641      672      +31     
- Misses         50       78      +28     
- Partials       93       94       +1     
Flag Coverage Δ
e2etests 79.62% <97.26%> (-2.14%) ⬇️
unittests ?

Flags with carried forward coverage won't be shown. Click here to find out more.

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

Copy link
Collaborator

@jennydaman jennydaman left a comment

Choose a reason for hiding this comment

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

Thank you so much for the contribution! I look forward to using it myself.

Would you try recreating some examples? If not, we can go ahead and merge this and I'll do that in the future.

src/NiivueMutator.ts Outdated Show resolved Hide resolved
@thangngoc89
Copy link
Author

@jennydaman sorry for the late response. I'll find an official niivue example and add a react example for it in the next few days

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.

2 participants