-
Notifications
You must be signed in to change notification settings - Fork 1
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
273 - Editing an experiment tag leads to blank page #314
Conversation
* #develop changed some styling * Revert "#develop changed some styling" This reverts commit a85b1f8. * #290 added indicator cord image and label --------- Co-authored-by: afonso pinto <[email protected]>
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.
Looks good but there are still a few problems:
- the context menu changing position sometimes:
2023-10-30.17-29-01.mp4
If it's easier, I would be ok with the context menu being closed on modal close
- There's currently no validation on the required fields
applications/portal/frontend/src/components/home/ExplorationSpinalCordDialog.tsx
Outdated
Show resolved
Hide resolved
applications/portal/frontend/src/components/home/ExplorationSpinalCordDialog.tsx
Show resolved
Hide resolved
return Response(status=status.HTTP_204_NO_CONTENT) | ||
except Exception as e: | ||
# This will return any error messages if the tag deletion fails | ||
return Response({"error": str(e)}, status=status.HTTP_400_BAD_REQUEST) |
Check warning
Code scanning / CodeQL
Information exposure through an exception
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.
Trying to edit a experiment twice (in sequence) doesn't seem to be updating the dashboard:
https://github.com/MetaCell/salk-interactive-atlas/assets/19196034/1d107be8-40ec-458b-9c26-df993a4727b2
const handleAction = async () => { | ||
setIsLoading(true) | ||
const errorsSet = await getValidationErrors() | ||
console.log("errorset: ", errorsSet) |
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.
This can be deleted
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 also noticed that your fix of the merge conflict is adding back some files that were removed purposely (namely data/residential_populations and objs files) which is not correct. Might be easier to branch from develop again and add your commits on top of that
freeSolo | ||
limitTags={3} | ||
renderTags={(value, getTagProps) => | ||
value.map((option, index) => ( | ||
<Chip key={index} style={{...tagsColorOptions[index]}} onDelete={() => console.log(option)} label={option} {...getTagProps({ index })} /> | ||
<Chip key={index} style={{ ...tagsColorOptions[index] }} onDelete={() => console.log(option)} label={option} {...getTagProps({ index })} /> |
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.
This onDelete callback looks strange
@@ -89,7 +89,7 @@ export const App = (props: any) => { | |||
{loading ? <Loader/> : ( | |||
<> | |||
<ProtectedRoute exact={true} path="/"> | |||
<HomePage latestExperimentId={latestExperimentId} /> | |||
<HomePage latestExperimentId={latestExperimentId} onExperimentChange={(id: string) => setLatestExperimentId(id)} /> |
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.
My guess is that the onExperimentChange callback is the reason why the bug I reported is happening.
When we change an experiment twitce, on the second change the id of the latest experiment doesn't change therefore there's no re-render of the dashboard.
replaced by #317 |
Closes #273
Implemented solution:
Displayed dialog window on card level, implemented on change function for each instance (name, desciption, tags), which changes the state, after calling the updateExperiment api we change the experiment then we call the function to call the experiments list again