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

Use wdk record page for study details for user datasets in eda #1327

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

Conversation

dmfalke
Copy link
Member

@dmfalke dmfalke commented Feb 17, 2025

This PR updates EDA to use the WDK record page for study details for analyses made from user datsets.

Resolves #1337

@dmfalke dmfalke force-pushed the eda_user-dataset__wdk_study_detail branch from 069a5cc to 969f25b Compare February 18, 2025 21:31
@asizemore
Copy link
Member

Types looking good. Can see expected properties in the console.

Screen Shot 2025-02-27 at 5 15 12 PM

@asizemore
Copy link
Member

Made a component for the publications part. It's not pretty, but it's functional!

Some considerations

  • Since publications are optional, I started with 0 publications input sections. If we start with 1 open already, then the asterisk for pubmedid makes it seem required, but actually it's not if we don't have any publications.
  • Give users ability to delete a publication they didn't mean to add.
Screen.Recording.2025-03-05.at.11.19.07.AM.mov

Will do a little more styling later today. Once I get this component looking good, I can just reuse the logic+styling for all the rest of the new properties.

@asizemore
Copy link
Member

@dmfalke before I get too far, what do you think of the Publications strategy? Basically i've made a PublicationInput component that we can add as many times as necessary. It seems to function well so far. I didn't put too much effort into styling but it might be good enough for now? Anyways, wanted to get a quick sanity check on this before moving too much further with extracting the styles to a css file and just generally cleaning up a bunch (i'm noticing we could benefit from more white space in multiple as well!). If this looks good to you, I'll do the same for the other new props where we're adding possibly multiple items in an array.

Screen.Recording.2025-03-05.at.5.09.41.PM.mov

@dmfalke
Copy link
Member Author

dmfalke commented Mar 5, 2025

I think this looks really nice! I would plan to move forward with this approach.

@asizemore
Copy link
Member

Looking good, all the new fields are there. I didn't add createdOn because that seemed like something that a user shouldn't be able to set... Anyways, I think I can consolidate and organize the code a little better, and there's one weird behavior i haven't figured out yet - When trying to remove a contact that precedes an empty contact, the intended contact will get removed in the object, but the ui still shows the non-empty contact on the page. See last little bit of video.

Screen.Recording.2025-03-07.at.5.54.27.PM.mov

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.

User datasets: Add new metadata fields to upload form
3 participants