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

NickAkhmetov/CAT-776 Dataset Relationships diagram #3481

Merged
merged 20 commits into from
Jul 30, 2024

Conversation

NickAkhmetov
Copy link
Collaborator

@NickAkhmetov NickAkhmetov commented Jul 24, 2024

Summary

This PR implements the dataset relationships diagram. It is currently displayed under the summary of raw datasets with at least one descendant in provenance that is not a publication or image pyramid. In the future, its position in the page will match the figma designs.

I also included other minor fixes:

  • Since the status icons were being defined at runtime, they caused Storybook to flicker.
  • The table of contents' section generation code no longer generates sections with the same node key/ID for cases where one dataset has multiple descendants with the same pipeline name. It also correctly handles cases where an invalid descendant has no pipeline name.
  • I disabled the Graph tab in the provenance view for datasets that respond to provdata requests with empty objects, as this caused pagewide crashes if clicked on.

TODOs

  • Update summary section of dataset page view - Holding off on this work to avoid interfering with entity header PR after conversation with John
  • Integrate the dataset relationships component with the rest of the page
    • Currently appended to the bottom fold of the summary
  • Include arrows in diagram edges
  • Add storybook story for real prov json example
  • Add test coverage for prov -> node conversion logic (if possible)
  • Make dataset nodes link to the appropriate hash code
  • Update to a more visually pleasing layouting approach (if possible)
  • Figure out why the entity nodes sometimes don't appear though the pipeline ones do
    • Appears to have had something to do with dynamically generated styled components - have not been able to reproduce this since fixing that issue.
  • Handle various edge cases
    • Publication descendants are not displayed
    • Image pyramid descendants are not displayed
    • Crashes on invalid/error dataset pages
  • Fix storybook/jest again

Design Documentation/Original Tickets

Figma

JIRA Subtask

Testing

  • Added Storybook stories
  • Tested generated diagrams locally with 6df2f796ad72307d04dc94d688b725c5 (CODEX) and a6a35e2441c6196e74d36c95261df592 (Visium)

Screenshots/Video

image

image

image

Checklist

  • Code follows the project's coding standards
    • Lint checks pass locally
    • New CHANGELOG-your-feature-name-here.md is present in the root directory, describing the change(s) in full sentences.
  • Unit tests covering the new feature have been added
  • All existing tests pass
  • Any relevant documentation in JIRA/Confluence has been updated to reflect the new feature
  • Any new functionalities have appropriate analytics functionalities added

Additional Notes

Please specify any additional information or context relevant to this PR.

@NickAkhmetov
Copy link
Collaborator Author

Got layouting working by adding a consistently sized wrapper to the nodes:
image

@NickAkhmetov NickAkhmetov marked this pull request as ready for review July 26, 2024 20:09
Copy link
Collaborator

@john-conroy john-conroy left a comment

Choose a reason for hiding this comment

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

Very clean! A few questions mostly for my benefit, feel free to ignore any.

alignItems="center"
justifyContent="center"
>
{Icon && <Icon color="primary" fontSize="0.75rem" width="0.75rem" height="0.75rem" />}
Copy link
Collaborator

Choose a reason for hiding this comment

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

What necessitated both fontSize and width + height?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I believe this was an artifact from troubleshooting the issue with dynamic status icons causing flickering, I'll remove the unnecessary width/height props.

borderRadius={rounded ? '1rem' : 0}
maxWidth="18rem"
bgcolor={bgColor}
boxShadow="0px 0px 2px 0px rgba(0, 0, 0, 0.14), 0px 2px 2px 0px rgba(0, 0, 0, 0.12), 0px 1px 3px 0px rgba(0, 0, 0, 0.20)"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Custom box shadow that wasn't in the theme, I imagine?

Comment on lines +69 to +75
<Box height="4.125rem" display="flex" alignItems="center">
<Stack
direction="column"
px={2}
py={1}
borderRadius={rounded ? '1rem' : 0}
maxWidth="18rem"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why these rem values?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Mainly to match the measurements from the Figma designs; the outer Box is necessary to make the node positions line up (i.e. by making them all the same height without affecting the appearance of the content)

context/app/static/js/hooks/useSoftAssay.tsx Outdated Show resolved Hide resolved
@NickAkhmetov NickAkhmetov merged commit e8360ca into unified-datasets Jul 30, 2024
5 checks passed
@NickAkhmetov NickAkhmetov deleted the nickakhmetov/dataset-relationships branch July 30, 2024 13:09
john-conroy added a commit that referenced this pull request Aug 21, 2024
* Add changelog

* NickAkhmetov/CAT-677 Unified Prov Graph (#3458)

Co-authored-by: John Conroy <[email protected]>
Co-authored-by: Tabassum Kakar <[email protected]>
Co-authored-by: tkakar <[email protected]>
Co-authored-by: Austen Money <[email protected]>
fix table tabs (#3466)
Fix incorrect URL structure in gene search results (#3467)

* John conroy/update table of contents CAT-755 (#3473)

* Convert table of contents to typescript

* Convert route to ts and add boundaries

* Fix styling so top works

* Add icons map

* Fix responsiveness

* Handle sub links

* Update marker color

* Style table of contents

* Add icons to table of contents

* Add processed datasets for table of contents

* Add loading state

* Adjust padding per Tiffany feedback

* Update style per Tiffany's feedback

* Open toc accordions by default

* Add visualization to processed data sections in toc

* Add processed sections and refactor

* Update changelog

* Update organ page to use layout

* Switch to derived data per tiffany's feedback

* Use height props

* Pull utils/hooks/types into files

* Add a use callback for toc click handler

* Pull find active index into hook

* Consolidate styles

* Use system props

---------

Co-authored-by: John Conroy <[email protected]>

* NickAkhmetov/CAT-776 Dataset Relationships diagram (#3481)

* John conroy/summary bar (#3501)

* Remove placeholder for undefined items

* Create entity header action buttons

* Add workspaces button

* Add copy button

* Always show entity header

* Add basic view select chips

* Fix opacity transition

* Animation progress

* Stash

* Add check icon to view select

* Add datasets relationship graph to summary bar

* Delete unused gene components

* Update summary components

* Different heights for summary and diagram

* Show entity header content when expanded

* Use position sticky

* Fix divider

* Remove version select

* Add citation and consortium to summary for datasets

* Clamp description in expanded summary

* Only show view buttons on large desktops

* Show mapped status

* Dedupe flask data and entity store assay metadata

* Add icons to summary

* Split out cases into components

* Add icons

* Add paper for relationship diagram

* Show summary view for more entities

* Integrate publication summary

* Fix publication summary

* Fix undefined

* Fix tests

* Fix offsets

* Remove fit view

* Fix comments from review

* Fix data-testid propogation

---------

Co-authored-by: John Conroy <[email protected]>

* NickAkhmetov/CAT-777 Processed Data section (#3495)

* make sure sample metadata doesn't get lost

* hide empty dataset relationships box on dataset pages without processed descendants

---------

Co-authored-by: John Conroy <[email protected]>
Co-authored-by: Nikolay Akhmetov <[email protected]>
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