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-736 Convert sample detail page to TSX #3592

Merged
merged 2 commits into from
Nov 5, 2024

Conversation

NickAkhmetov
Copy link
Collaborator

Summary

This PR converts the sample detail page to TypeScript. Minimal changes were necessary, as most detail page components were converted during the unified dataset detail page push.

Design Documentation/Original Tickets

https://hms-dbmi.atlassian.net/browse/CAT-736

Testing

I viewed several sample pages without encountering errors.

Screenshots/Video

N/A

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

I wonder if we can enhance the EUI links on sample pages to link directly to the sample in question, e.g.: https://portal.hubmapconsortium.org/browse/sample/e5f2cb399e7e8885134c1b1a2b6acc9a
The ccf eui link in the tissue section just takes the user to the main ccf eui page in the portal, i.e. https://portal.hubmapconsortium.org/ccf-eui; it seems like it would be a relatively straightforward enhancement for us to pass in a URL param to preselect the appropriate sample since it's not clear how one would find the sample in the EUI otherwise.

@@ -30,12 +37,13 @@ function SampleDetail() {
entity_type,
descendant_counts,
ancestor_ids,
mapped_data_access_level,
} = entity;

const [entities, loadingEntities] = useEntitiesData([uuid, ...ancestor_ids]);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think useEntitiesData accepts a doc as a generic. Could that prevent the assertions below?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good catch, I didn't realize that generic was there 👍🏻

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.

Thanks!

@NickAkhmetov NickAkhmetov merged commit df33e55 into main Nov 5, 2024
8 checks passed
@NickAkhmetov NickAkhmetov deleted the nickakhmetov/cat-736-convert-sample-detail-page branch November 5, 2024 19:26
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