cBioPortal / clinical-timeline

Use d3-timeline to visualize clinical data
GNU Lesser General Public License v3.0
22 stars 18 forks source link

changes have been made to exportTimeline.js to fix issue #80 #108

Closed ayush-k closed 7 years ago

ayush-k commented 7 years ago

Signed-off-by: ayush kataria ayushkataria@ayushs-MacBook-Pro.local

What? Why?

Fix #80

Changes proposed in this pull request:

Checks

Any screenshots or GIFs?

If this is a new visual feature please add a before/after screenshot or gif here with e.g. GifGrabber. clinical-timeline (1).pdf clinical-timeline 17

Notify reviewers

@inodb

rohangoel96 commented 7 years ago

Hi @ayush-k , thank you for your contribution. Unfortunately, I still can't see the ticks on the timeline axis (going by the screenshot you attached).

According to me what @inodb meant is that we need the screenshot to have ticks in the timeline something like: screenshot from 2016-12-23 16 16 35

Note: Presently your screenshot shows a thick solid black line without ticks. screenshot from 2016-12-23 16 18 04

ayush-k commented 7 years ago

I wasn't able to do that rather I added another time line(the overview one with ticks) at the bottom to compensate for the irksome timeline above.

On Dec 23, 2016 4:18 PM, "Rohan Goel" notifications@github.com wrote:

Hi @ayush-k https://github.com/ayush-k , thank you for your contribution. Unfortunately, I still can't see the ticks on the timeline axis (going by the screenshot you attached).

According to me what @inodb https://github.com/inodb meant is that, we need the screenshot to have ticks in the timeline somthing like: [image: screenshot from 2016-12-23 16 16 35] https://cloud.githubusercontent.com/assets/8400677/21452199/4c79af30-c92b-11e6-86bb-394a65428ed8.png

Note: Presently your screenshot shows a thick solid black line without ticks. [image: screenshot from 2016-12-23 16 18 04] https://cloud.githubusercontent.com/assets/8400677/21452213/6a656804-c92b-11e6-8f59-89f8fd98f6ff.png

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/cBioPortal/clinical-timeline/pull/108#issuecomment-268972171, or mute the thread https://github.com/notifications/unsubscribe-auth/AGQlKeClE9vO94eMVEJ-HybDeOHwu5s9ks5rK6b6gaJpZM4LUtfy .

inodb commented 7 years ago

Thanks so much for giving this a shot @ayush-k. @rohangoel96 is correct that we would like to show the proper ticks at the top. The main way this feature is used is for researchers/doctors to show the timeline in publications or share them with their colleagues. The overview timeline at the bottom is unfortunately not super useful to show in such a format. It tells you, you are not seeing the complete timeline, which is only insightful when you can actually interact with it. Hope you are willing to work more on this! I really appreciate your effort! Thank you!

ayush-k commented 7 years ago

Definitely , I will try to get the original issue sorted

inodb commented 7 years ago

Thanks a bunch! Fixed in: https://github.com/cBioPortal/clinical-timeline/commit/a3ca4e3f12deb20b67c0dea308dd16b2eafb6f19