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: Split up cohort view general design into two sections: General … #3760

Conversation

connoratrug
Copy link
Contributor

@connoratrug connoratrug commented May 16, 2024

…design and Population and add some missing data items

Closes #3609

see issue

…design and Population and add some missing data items

Closes #3609
Copy link
Member

@mswertz mswertz left a comment

Choose a reason for hiding this comment

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

code looks good @BrendaHijmans please approve after functional review.

P.S. I notice that if there are ontoogies without nesting then we get a lot of empty space (to hold the arrow down that is not there). Would that be something to fix? I.e. that we don't indent when there is no reason? Or that we introduce another icon for items that have no children?

image

@connoratrug
Copy link
Contributor Author

connoratrug commented May 17, 2024

code looks good @BrendaHijmans please approve after functional review.

P.S. I notice that if there are ontoogies without nesting then we get a lot of empty space (to hold the arrow down that is not there). Would that be something to fix? I.e. that we don't indent when there is no reason? Or that we introduce another icon for items that have no children?

image

Yes i noticed, this is actually a bit tricky , as sometimes the tree is used in a larger list where you would like the items to left-align, suggest to handle this an a separate issue, we could add a prop to render the tree as list if max-depth = 1 , but then you would need to check all tree use cases to see if it should do this

Even in the case show here there is something to be said for having a left scannable edge

Copy link
Contributor

@BrendaHijmans BrendaHijmans left a comment

Choose a reason for hiding this comment

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

Can you put 'Population' into a separate panel? It seems inconsistent to keep two headings in one panel.

As described in #3609, can you name 'Population oncology topology' 'ICDO site' and cab you name 'Population oncology morphology' 'ICDO morphology'?

Can you rename 'Age group at inclusion' to 'Poplation age groups'? This was agreed upon by Eleanor and Kim.

@EleanorHyde-UMCG
Copy link
Contributor

EleanorHyde-UMCG commented May 21, 2024

Can you put 'Population' into a separate panel? It seems inconsistent to keep two headings in one panel.

As described in #3609, can you name 'Population oncology topology' 'ICDO site' and cab you name 'Population oncology morphology' 'ICDO morphology'?

Can you rename 'Age group at inclusion' to 'Poplation age groups'? This was agreed upon by Eleanor and Kim.

In addition to what Brenda added above, could you also please do the following as well?

  • check you've not accidentally removed the "Design schematic" field - I can't see it currently in dev and this may be a data issue, but in acc it should still be visible
  • add the 'Publications' field under the Design paper field
  • add 'Population disease' under 'Main medical condition'
  • add the following fields under 'Access Conditions':
    • Release type
    • Linkage possibility description
    • Data access fee
    • Prelinked
    • Data holder
    • Data use conditions

Thanks!

@connoratrug

This comment was marked as resolved.

@BrendaHijmans

This comment was marked as resolved.

Copy link

sonarcloud bot commented May 22, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@connoratrug connoratrug merged commit 13e400c into master May 22, 2024
4 of 6 checks passed
@connoratrug connoratrug deleted the 3609-split-up-cohort-view-general-design-into-two-sections-general-design-and-population-and-add-some-missing-data-items branch May 22, 2024 11:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants