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-1078 Fix unselectable collections tabs #3688

Merged
merged 3 commits into from
Feb 12, 2025

Conversation

NickAkhmetov
Copy link
Collaborator

@NickAkhmetov NickAkhmetov commented Feb 11, 2025

Summary

This PR makes unselectable collections tabs clickable, so that the content of their tab panel changes accordingly.

Design Documentation/Original Tickets

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

Testing

Manual testing

Screenshots/Video

Screen.Recording.2025-02-11.125752.mp4

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

@NickAkhmetov
Copy link
Collaborator Author

NickAkhmetov commented Feb 11, 2025

Maybe revisiting the fetching strategy for collections data is necessary - i.e., hoisting the fetch of all collection-related data to the container to pre-filter which tabs should and shouldn't be displayed before the first render of the Tabs component altogether

This ended up being the fix 😄

@NickAkhmetov NickAkhmetov marked this pull request as ready for review February 11, 2025 18:02
@@ -11,6 +14,7 @@ function buildCollectionsWithDatasetQuery(datasetUUIDs: string[]): SearchRequest
'datasets.uuid': datasetUUIDs,
},
},
_source: ['uuid', 'title', 'hubmap_id', 'datasets'],
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need everything in datasets?

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'll revert this, this was a leftover

austenem
austenem previously approved these changes Feb 11, 2025
Copy link
Collaborator

@austenem austenem left a comment

Choose a reason for hiding this comment

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

LGTM, just one question.

<TabPanel value={value} index={index}>
<OutlinedAlert severity="info">The raw dataset is not referenced in any existing collections.</OutlinedAlert>
</TabPanel>
);
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is pre-existing, but what is the case for this panel being returned? It's my understanding that if the dataset is not referenced by a collection, there shouldn't be any tab present for that dataset.

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've rearranged the logic so that the collections data is fetched once for all datasets on the page, then sorted into buckets by dataset on the client side. The resulting map starts with an empty entry for the raw dataset; all other datasets with no matching collections are not included whatsoever. With this approach, it's known that the only reason why there would be a panel displayed with no collections data to show for it is because it is the primary dataset.

@NickAkhmetov NickAkhmetov changed the title NickAkhmetov/CAT-1078 Stopgap fix for unselectable collections tabs NickAkhmetov/CAT-1078 Fix unselectable collections tabs Feb 11, 2025
@NickAkhmetov NickAkhmetov merged commit e96d25d into main Feb 12, 2025
8 checks passed
@NickAkhmetov NickAkhmetov deleted the nickakhmetov/cat-1078-collections-section-tabs branch February 12, 2025 20: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.

3 participants