CouncilDataProject / seattle_v2

Website for interacting with Seattle's instance of CDP
https://councildataproject.github.io/seattle
BSD 3-Clause "New" or "Revised" License
5 stars 6 forks source link

[GH-51]feature/add-timestamped-speaker-turns #57

Closed tohuynh closed 4 years ago

tohuynh commented 4 years ago

Pull request recommendations:

Thanks for contributing!

evamaxfield commented 4 years ago

Before I review, how did you test this without any speaker turns transcripts existing in the database? / How can I test this haha

evamaxfield commented 4 years ago

Also from the changes, am I assuming correctly that the same code is used for scrolling to position as is the EventTranscriptSearch? So when you click on a result in the Event Transcript Search it will also scroll to that position in the full transcript as well?

tohuynh commented 4 years ago

Also from the changes, am I assuming correctly that the same code is used for scrolling to position as is the EventTranscriptSearch? So when you click on a result in the Event Transcript Search it will also scroll to that position in the full transcript as well?

I'm not following the question. There is no automatic scrolling in EventTranscriptSearch. It just renders the sentences that contains the searchText.

evamaxfield commented 4 years ago

Also from the changes, am I assuming correctly that the same code is used for scrolling to position as is the EventTranscriptSearch? So when you click on a result in the Event Transcript Search it will also scroll to that position in the full transcript as well?

I'm not following the question. There is no automatic scrolling in EventTranscriptSearch. It just renders the sentences that contains the searchText.

Right but the onclick of any of the results, does that onclick now also scroll to the full transcript position of that clicked result?

tohuynh commented 4 years ago

Right but the onclick of any of the results, does that onclick now also scroll to the full transcript position of that clicked result?

Oh do you mean the onclick of the play button?

evamaxfield commented 4 years ago

Right but the onclick of any of the results, does that onclick now also scroll to the full transcript position of that clicked result?

Oh do you mean the onclick of the play button?

Yep!

Sidenote: GitHub should really look into a thread system for conversations.

tohuynh commented 4 years ago

Right but the onclick of any of the results, does that onclick now also scroll to the full transcript position of that clicked result?

Oh do you mean the onclick of the play button?

Yep!

Sidenote: GitHub should really look into a thread system for conversations.

No, it doesn't do that right now. That's a good idea. This can be done I think through sending a custom-event and have EventTranscript listen for that event and then scroll to right portion.

tohuynh commented 4 years ago

New changes removed react-virtualized components from EventTranscript and uses element.scrollIntoView to do the scrolling.

It scrolls to the transcript portion if there's a videoTimePoint in the URL, or the user clicks a transcript portion's play button in Search Transcript.