Skip to content

Adjust content search when switching between transcripts - #532

Merged
Dananji merged 5 commits into
mainfrom
refine-content-search
Jun 27, 2024
Merged

Adjust content search when switching between transcripts#532
Dananji merged 5 commits into
mainfrom
refine-content-search

Conversation

@Dananji

@Dananji Dananji commented Jun 21, 2024

Copy link
Copy Markdown
Collaborator

Refinements to feature added in #502

This PR includes the following refinements to content search:

  • Use cached search results when switching between transcripts in the same Canvas without changing the query
  • Preserve search counts when switching to a transcript where search is disabled
  • Update search results counts properly when switching between transcripts
  • Clear search counts when current search is cleared
  • Display hit counts correctly when there are multiple hits in a single cue
  • General refactoring to parts of code related to transcript component

@Dananji
Dananji marked this pull request as draft June 24, 2024 13:25
@Dananji
Dananji marked this pull request as ready for review June 24, 2024 19:16

@cjcolvar cjcolvar left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did a little testing and search feels improved. I ran into a couple inconsistencies:

  • highlighting words within other words (e.g. you and your)
  • phrase searching correctly highlights but hit count is 0 (e.g. feeling better)

Comment thread src/services/transcript-parser.js Outdated
@Dananji
Dananji marked this pull request as draft June 26, 2024 17:59
@Dananji
Dananji force-pushed the refine-content-search branch from d2a7178 to 5827fd3 Compare June 26, 2024 18:00
@Dananji
Dananji force-pushed the refine-content-search branch from 5827fd3 to 3e24639 Compare June 26, 2024 18:08
@Dananji
Dananji marked this pull request as ready for review June 26, 2024 18:08
@Dananji
Dananji marked this pull request as draft June 27, 2024 13:02
@Dananji
Dananji force-pushed the refine-content-search branch from 08ae4d3 to 0eb40ec Compare June 27, 2024 14:22
@Dananji
Dananji marked this pull request as ready for review June 27, 2024 14:22

@masaball masaball left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I can tell this looks good.

Comment thread src/services/transcript-parser.js Outdated
@Dananji
Dananji force-pushed the refine-content-search branch from 5a1a27f to 0a96e1b Compare June 27, 2024 15:51
@Dananji
Dananji merged commit 0322404 into main Jun 27, 2024
@Dananji
Dananji deleted the refine-content-search branch June 27, 2024 15:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants