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

Refactor: deload updating mydata from slope selector #185

Draft
wants to merge 3 commits into
base: documentation/create-vignettes
Choose a base branch
from

Conversation

m-kolomanski
Copy link
Collaborator

@m-kolomanski m-kolomanski commented Jan 31, 2025

TODO: I have messed up with the base branch, update base when #184 is merged
TODO: Wait on #177 to be merged

Issue

Addresses #121, #144

Description

  • Updates slope_selector so that it no longer updates mydata object. Apply button has been removed. Instead, the user should click on Run NCA in order to update the results, which is handled by nca.R.
  • Adds warning stating that settings has been changed and the analysis must be re-run.
  • In addition, table for viewing active selections/exclusions has been modularized and only updates when results have been recalculated.

How to test

TBA

Contributor checklist

  • Code passes lintr checks
  • Code passes all unit tests
  • New logic covered by unit tests
  • New logic is documented
  • Package version is incremented

Notes to reviewer

TBA

@m-kolomanski m-kolomanski changed the base branch from main to documentation/create-vignettes January 31, 2025 09:35
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.

1 participant