search

informatics-isi-edu / openseadragon-viewer

2D viewer with openseadragon
Apache License 2.0
5 stars 2 forks source link

Merging the codes #12

Closed vipul-21 closed 4 years ago

vipul-21 commented 4 years ago

This PR is dependent on the chaise PR. It resolves the issues #11. Test cases : https://github.com/informatics-isi-edu/chaise/wiki/Viewer-regression-tests

iamyle0710 commented 4 years ago

Changes are good overall, just found some minor differences when I tested the cases.

[Zoom and Pan]

  1. thumbnail shown in the current version, but not in the staging version(Not sure if this is an issue)

[Scale bar]

  1. Scalebar is shown in the staging version, not in the current version

[Screenshots]

  1. changes are good. It works as in the staging version
    • checked with following file formats: czi, tiff from image server, jpg

[Multi-channels]

  1. when I adjust the contrast to 80, it doesn't seem to have same effect as in the staging version
hongsudt commented 4 years ago

All these issues need to be addressed especially the scale bar..

Under zoom and pan, we should figure out why the thumbnail is shown; but more importantly the color in the thumbnail is different from the actual image.. This is bad since it is confusing.

From: MingYi Lin notifications@github.com Sent: Thursday, March 26, 2020 3:06 PM To: informatics-isi-edu/openseadragon-viewer openseadragon-viewer@noreply.github.com Cc: Subscribed subscribed@noreply.github.com Subject: Re: [informatics-isi-edu/openseadragon-viewer] Merging the codes (#12)

Changes are good overall, just found some minor differences when I tested the cases.

[Zoom and Pan]

  1. thumbnail shown in the current version, but not in the staging version(Not sure if this is an issue)

[Scale bar]

  1. Scalebar is shown in the staging version, not in the current version

[Screenshots]

  1. changes are good. It works as in the staging version

    • checked with following file formats: czi, tiff from image server, jpg

[Multi-channels]

  1. when I adjust the contrast to 80, it doesn't seem to have same effect as in the staging version

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHubhttps://github.com/informatics-isi-edu/openseadragon-viewer/pull/12#issuecomment-604711513, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AADFIDFCWVMYF63OERH27M3RJPGVRANCNFSM4LTDYFXQ.

vipul-21 commented 4 years ago

I've updated the PR.

  1. Resolved the thumbnail issue.
  2. In the scale issue, for xml on staging the response include meterScaleInPixels. But in the dev, it does not include the meterScaleInPixels that is why it is not shown on dev. @svoinea Do you know why ?
  3. Multichannel issue with respect to contrast resolved.
svoinea commented 4 years ago

I have tested the cases specified on the wiki https://github.com/informatics-isi-edu/chaise/wiki/Viewer-regression-tests page using my home public_html directory with:

Two cases need attention:

vipul-21 commented 4 years ago

@svoinea The case #2: I've fixed the situation. case #13: I checked the production, it seems to work in the same manner here as well.

svoinea commented 4 years ago

I have retested issues #2 and #13 and they work as expected.

vipul-21 commented 4 years ago

@hongsudt I have updated the PR with the cross on the channel list.

svoinea commented 4 years ago

I have applied the latest changes on my public_html directory and the test case #29 looks good to me.

Test case #31 remains open, the channel button remaining enabled, and showing No channels found when you click it.