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

PORTALS-3385: [CCKP Homepage_V1] Add icons to CCKP cards for data types #1620

Merged
merged 6 commits into from
Mar 6, 2025

Conversation

kianamcc
Copy link
Collaborator

@kianamcc kianamcc commented Mar 5, 2025

Jira: https://sagebionetworks.jira.com/browse/PORTALS-3385?atlOrigin=eyJpIjoiNDVlMmNiODdkZmU0NGVkZGFiYmMwNmFiNzhiOTY1YjgiLCJwIjoiaiJ9

IconList:
Screenshot 2025-03-05 at 2 30 27 PM

Created a handleIconClick function to implement this functionality.

Will create a PORTALS-3385-V2 branch to add this feature to CCKP when data is available and add a link to that PR in the ticket.

@kianamcc kianamcc marked this pull request as draft March 5, 2025 22:47
const queryContext = useQueryContext()
const { addValueToSelectedFacet } = queryContext

const handleIconClick = (dataType: 'string') => {
Copy link
Member

Choose a reason for hiding this comment

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

Function is recreated on rerender (I think). Is this a good place for a useCallback? (It's unclear to me if IconSvg is going to be re-rendered when this is recreated)

const facet: UniqueFacetIdentifier = {
columnName: 'dataType',
}
addValueToSelectedFacet(facet, dataType)
Copy link
Member

Choose a reason for hiding this comment

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

I verified that addValueToSelectedFacet in TableQueryReducerActions handles the case when the caller attempts to add the same facet value more than once (duplicate value is not pushed). So this should be fine!

@kianamcc kianamcc marked this pull request as ready for review March 6, 2025 20:38
@kianamcc kianamcc merged commit 2f17a74 into Sage-Bionetworks:main Mar 6, 2025
24 of 25 checks passed
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