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
266 stars 136 forks source link

Add data source selection to cs designer #2799

Closed anton-abushkevich closed 1 year ago

anton-abushkevich commented 1 year ago

Resolves #2798

chrisknoll commented 1 year ago

I also loaded the code and the tables are not rendering (probably due to removing closing tags from the compoennts:

image

Note: this above image looks broken to me, but you can open in new tab on the link to see it.

TitrS commented 1 year ago

I also loaded the code and the tables are not rendering (probably due to removing closing tags from the compoennts:

image

Note: this above image looks broken to me, but you can open in new tab on the link to see it.

I can't to reproduce this issue. Could you show your console.log?

chrisknoll commented 1 year ago

There are no errors on console, and there are no DOM elements in the document, here's the DOM for the tab containing the result.

I don't know why you can't reproduce? What version of Chrome are you running? I'm on Version 108.0.5359.125 (Official Build) (64-bit)

image

chrisknoll commented 1 year ago

Ok, I pushed up a fix tha tresolves this: the <datasource-select> needs to have the close tag, not the </> syntax. This is just a limitation when dealing with components.

I'm not ready to approve this because I'd like to investgate removing the notion of 'tabs' form the conceptSetStore (it's supposed to be something you may want to use outside of tabs). Also, I understand that you have events hooking into the dropdown selection such that when you change it, it should trigger a refresh, I'm wondering if this is a case where the dropdown selects a selected datasource, and we subscribe to the changes to that value (I recognize that this goes against comments I said elsewhere about avoiding subscriptions, so what you've done here may be the best way, I just need to think about it).

Lastly, iI noticed that the datasource dropdown can be different form tab-to-tab, ie: it can be Source1 on one tab, and you go to another to see Source2, but then you return to the firs ttab and it's still Source1. I would expect that if you change record counts, that those are switched across tabs. If we want the datasource shared, then the selected datasource should be done at the 'parent' level and passed into each component for changing, and when that value changes it should refresh the contents of the selected mode via refresh().

chrisknoll commented 1 year ago

I pushed a commit to resolve conflicts.

There is an issue: when I change the selected datasource on the included concepts tab, the included source code tab has the other datasource selected. I thought we said that we wanted to have the selected datasource 'shared' across the tabs. I'm speaking specifically about the embedded concept set editor.

anton-abushkevich commented 1 year ago

@chrisknoll I forgot to add parameter to embedded editor, sorry for that. Fixed, please review.

chrisknoll commented 1 year ago

All better now, thank you.