Add data source selection to cs designer#2799
Conversation
| lengthMenu: tableOptions.lengthMenu, | ||
| language: ko.i18n('datatable.language') | ||
| }"></faceted-datatable> | ||
| }"/> |
There was a problem hiding this comment.
I would avoid this change: KO can run into template parsing problems if you don't have explicit close tags. Please avoid style changes unless it's absolutely necessary.
| } | ||
|
|
||
| async refresh(mode) { | ||
| this.currentConseptSetTab(mode); |
There was a problem hiding this comment.
Is this necessary? the change to currentCnceptSetTab() should result in a call to refresh(mode), but then refresh mode wants to reset currentConceptSetTab(). What issue were you trying to fix when you added this?
There was a problem hiding this comment.
I use this.currentConseptSetTab in DataSourceComponent for reload table data.
| const identifiers = concepts.map(c => c.CONCEPT_ID); | ||
| try { | ||
| const data = await vocabularyService.getMappedConceptsById(identifiers); | ||
| await vocabularyService.loadDensity(data); |
There was a problem hiding this comment.
The source codes does not show record counts (because source codes aren't actual records, but show the relationship between a mapped source code). Does this PR change the 'Included soruce codes' view to display row counts?
chrisknoll
left a comment
There was a problem hiding this comment.
I wanted to get feedback from my comments before approving this change.
|
I also loaded the code and the tables are not rendering (probably due to removing closing tags from the compoennts:
Note: this above image looks broken to me, but you can open in new tab on the link to see it. |
…urce_selection_to_cs_designer
I can't to reproduce this issue. Could you show your console.log? |
|
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)
|
|
Ok, I pushed up a fix tha tresolves this: the 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(). |
…ch source selection
…urce_selection_to_cs_designer # Conflicts: # js/components/conceptset/included.html
|
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. |
…in embedded CS editor
|
@chrisknoll I forgot to add parameter to embedded editor, sorry for that. Fixed, please review. |
|
All better now, thank you. |
Resolves #2798