search

GlobalDataverseCommunityConsortium / dataverse-previewers

A collection of Datafile Previewers that can be configured to work with Dataverse
MIT License
13 stars 38 forks source link

add preview mode #24 #25

Closed pdurbin closed 4 years ago

pdurbin commented 4 years ago

For #24 (not sure if it closes it yet).

@qqmyers before you even consider merging this pull request, I have some meta questions. šŸ˜„

(I noticed you gave me push access to this repo so I didn't bother forking.)

Do you know why git diff and git commit shows weird line endings? Should I try to get rid of them? I'll attach some screenshots below. I hacked on retriever.js on a CentOS EC2 instance and then copied it down to my Mac to actually commit and push the branch.

git diff (I'm not used to seeing all these ^M's)

Screen Shot 2019-10-22 at 5 32 31 PM

git commit (looks relatively normal or at least consistent, but I don't usually a ^M on every line like this.)

Screen Shot 2019-10-22 at 5 33 04 PM

You also mentioned something at https://github.com/QualitativeDataRepository/dataverse-previewers/issues/24#issuecomment-545033058 about possibly disabling an API call. Do you want me to look into that or should that be an optimization we get in later?

Finally, things are looking pretty good on Chrome but on Firefox I'm getting a mysterious spinner. Do you know why? Screenshots below.

Chrome

Screen Shot 2019-10-22 at 5 42 16 PM

Firefox

Screen Shot 2019-10-22 at 5 43 03 PM

juancorr commented 4 years ago

Hi @qqmyers and @pdurbin , I have checked the original retriewers.js and another removing the ^M with the dostounix command and, both of therm works in our server. Here are the view with the original retriewers.js (with the ^M). It works for me in Linux (Firefox and Chromium) and W10 (Firefox, Chrome and Edge): http://oaimadrono.uned.es:8080/file.xhtml?persistentId=doi:10.21950/VJADUC/SXOIDE

imagen

adam3smith commented 4 years ago

I believe ^M indicates CLRF line endings and shouldn't be there. git with the default --global core.autocrlf true config option should convert these to proper unix linendings.

djbrooke commented 4 years ago

Thanks @qqmyers for the review!

qqmyers commented 4 years ago

FWIW - I just committed a change to remove all of the ^M chars. If you see them again, let me know. I think I've changed my setup to avoid adding them back.

pdurbin commented 4 years ago

Closing in favor of pull request #27