-
-
Notifications
You must be signed in to change notification settings - Fork 0
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
386 - Refactor MoveToDatasetButton / MoveToDatasetModal components #435
base: dev
Are you sure you want to change the base?
386 - Refactor MoveToDatasetButton / MoveToDatasetModal components #435
Conversation
…ion in RefinebioContext (during the initial load of the application - replacing the useToken hook) ,and adjusted the other hooks that utilize token
…er viist without activation if it doesn't exist) and use the side effect to sync the token state with acceptedTerms in RefinebioContext, remove the token generation logic from consumer hooks, rename the prop name 'params' to 'body' in interface s methods
…of use checkbox visibility)
… the usage of token/applyAcceptedTerms), and use the token state in hooks instesd (added a new hook useTriggerSubmit - decuple the useEffect until adding conetxt/hook for file download and download options etc)
… accordingly, use this new hook in RefeinbioContext provider and add waitForToken method, use waitForToken in useDatasetManager
… to 'endpoint' for clarity, (minor) remove obsolete interfaces from the api folder
…set in MoveToDatasetButton, rename state/variables for clarity, clean up props, optimize the action methods for the MoveToDatasetModal
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
…-and-modal-components
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.
I think we can move some logic out of the modal and just have it take a callback for when a value is selected or cancelled.
const pathname = '/download' | ||
const myDatasetTotalSamples = getTotalSamples(myDataset?.data) | ||
|
||
const getRedirect = (prefix) => { |
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.
const getRedirect = (prefix) => { | |
const downloadRedirectWithPrefix = (prefix) => { |
const handleReplace = async () => { | ||
await replaceSamples(dataset.data) | ||
getRedirect('Moved') | ||
} |
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.
const handleReplace = async () => { | |
await replaceSamples(dataset.data) | |
getRedirect('Moved') | |
} | |
const handlers = { | |
append: handleAppend, | |
replace: async () => { | |
await replaceSamples(dataset.data) | |
downloadRedirectWithPrefix('Moved') | |
} | |
} | |
const handleSelectAction = (action) => { | |
await handlers[action]() | |
// analytics | |
// close modal | |
} |
Issue Number
Closes #386
Base branch: PR #421
Purpose/Implementation Notes
I've used an early return to handle the case of no samples in My Dataset, and optimized the action methods (append, replace) of the "Move to Dataset" button.
Types of changes
Functional tests
N/A
Checklist
Screenshots
N/A