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

feat: get case control population sizes in modal #1177

Conversation

pieterlukasse
Copy link
Contributor

@pieterlukasse pieterlukasse commented Dec 16, 2022

Jira Ticket: VADC-371

New Features

  • show Case/Control populations sizes as well

Screenshot:
Screenshot 2022-12-16 at 12 00 49

Copy link
Contributor

@jarvisraymond-uchicago jarvisraymond-uchicago left a comment

Choose a reason for hiding this comment

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

Looks good, I tested locally and it works!

  • I had a question about UPDATE_SELECTED_HARE hard coded value
  • Should the storybook for the ConfigureGWAS be updated to show both a dichotomous outcome selection and a non-dichotomous outcome selection?
  • Had a nitpick about iterator as key that can probably be ignored since the linter doesn't seem to care.
  • Ran ESLINT-new to autoformat files, this included changes from Master. Then I updated SPRINT25 to be up-to-date with Master (https://github.com/uc-cdis/data-portal/tree/feat/vadc_sprint25), but these updates are showing as changed files in this PR now. I don't think that's right and I'm not sure why that is occurring.

@pieterlukasse pieterlukasse changed the base branch from feat/vadc_sprint25 to master December 16, 2022 17:51
@pieterlukasse pieterlukasse changed the base branch from master to feat/vadc_sprint25 December 16, 2022 17:56
@pieterlukasse pieterlukasse force-pushed the feat/get_case_control_population_sizes_in_modal branch from 28fabfb to 77c0bba Compare December 16, 2022 18:34
Copy link
Contributor

@jarvisraymond-uchicago jarvisraymond-uchicago left a comment

Choose a reason for hiding this comment

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

Really good!

...and extra header in storybook to help developer test different scenarios
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