Closed mcin-armintaheri-archive closed 6 years ago
@ArminTaheri this is awesome! Am I right that this should be on hold until https://github.com/aces/cbrain/pull/665 is merged?
Yes correct! The html files will not render otherwise. There is concern about the way I changed the userfile#content
action and I have written a potential solution that we can discuss at a cbrain meeting or something. I believe once that is resolved the niak viewer will be good to go!
Quick update on a bug:
If I switch from Niak viewer to directory viewer it doesn't work unless I remove the iframe
embedding the viewer. There are absolutely no errors or 404s or Javascript console errors anywhere. It's very strange and might warrant only showing the "open in a new window" link.
I should have mentioned this when I found the issue this morning.
So I've taken @prioux's advice and made the Niak viewer open in a new tab. I've also added a new userfile_controller action to query file collections directly so we do not need to modify any of the niak result files. https://github.com/aces/cbrain/pull/666
There is still hope to have in embedded viewer without using an iframe
if I use the nokogiri
gem to parse out the head and body of the html files and render them directly in the niak_viewer
view. This should fix the issues with switching to directory viewer but I will only do this if @glatard or @poquirion feel it's worth the time to have an embedded viewer.
Hello @ArminTaheri, thank for doing that!
I think having no iframe would be really good. Bigger brain for QC is a plus.
+1 for separate tab.
Ah quick question about QC reports. Are they currently generated as HTML by Niak? My test Niak result only seems to have PDFs. If there is a newer NIAK result with HTML files for QC I'd love to have one of those!
There is also the option of rendering a table for the QC .csv
file in report/
QC report should live under the "report" subfolder of the preprocessing outputs. I will let @poquirion comment on the correct version.
I have good news, @pbellec, @poquirion, @glatard! aces/cbrain#666 has been merged and I believe it will be deployed today. There should be no problem merging this pull request as I've tested the NIAK viewer works at least locally.
I will wait for advice on the QC report viewer in the mean time.
Merging this since, awesome work @ArminTaheri!
I have fixed the niak viewer but there are a couple of hacky changes in content loader that @prioux seems unsure about.