search

OHDSI / Atlas

ATLAS is an open source software tool for researchers to conduct scientific analyses on standardized observational data
http://atlas-demo.ohdsi.org/
Apache License 2.0
258 stars 126 forks source link

Add conceptset preview #2792

Closed anton-abushkevich closed 1 year ago

anton-abushkevich commented 1 year ago

Resolves #2791

anton-abushkevich commented 1 year ago

@chrisknoll You are absolutely right. Please review the last commit - I added copying of existing CS items, instead of including them into the preview by reference.

Please note the ConceptSetItem.js changes. It turns out, that when creating a new object on the basis of existing one, the observable fields isExcluded, includeDescendants and includeMapped were always true, because for example data.isExcluded is a reference to a function, and in the expression data.isExcluded || false interpreted as true, so the result was always true. I fixed it by adding a call to extract the values from there observables, like data.isExcluded && data.isExcluded() || false.

chrisknoll commented 1 year ago

The onPreview param is only being set in search.js but if you use the preview button within a concept set editor, you get an error.

My concern here is that the preview behavior seems to be standard across any context (show a modal with the concept set editor with the clone) so could this behavior be embedded inside the ConceptAddBox component?

If it makes sense to have the component hosting the ConceptAddBox component provide their own impelementation of 'onPreview' then what you have works, but I was just curious if there will be a need to have context-specific implementations. Your implementation does provide that flexiblity, but it does mean we need to make sure that search, included concept, source concept tabs all provide a handler for onPreview param.

anton-abushkevich commented 1 year ago

@chrisknoll Do you think we need the preview in places other than the Search page? If yes, than sure - I can move preview handling into the ConceptAddBox or make some wrapper around it and move preview functionality there. For now I added a check - the Preview button is hidden if no preview handler provided.

Also I fixed multiple CS stores behavior - please review.

anthonysena commented 1 year ago

@anton-abushkevich from discussion today on the Atlas WG, we feel that having such a concept set preview would be useful wherever we work with Concept Sets in the user interface. So if we could modify this PR to have this capability, that would be a great addition to this enhancement.