Skip to content

Clue 187 read only highlight sparrow#2621

Open
eireland wants to merge 3 commits intomasterfrom
CLUE-187-read-only-highlight-sparrow
Open

Clue 187 read only highlight sparrow#2621
eireland wants to merge 3 commits intomasterfrom
CLUE-187-read-only-highlight-sparrow

Conversation

@eireland
Copy link
Contributor

@eireland eireland commented Jul 31, 2025

Made tracking highlight boxes in text tile state instead of text content model. Sparrows are now appearing in both edit and read-only views. Sparrows are drawing correctly in edit view, but not in read only view. The read only view draws the sparrows as they look in the edit view, not where the highlight chips are in the read-only view.

@cypress
Copy link

cypress bot commented Jul 31, 2025

collaborative-learning    Run #16283

Run Properties:  status check passed Passed #16283  •  git commit 74b5c9c18e: Remove console.logs
Project collaborative-learning
Branch Review CLUE-187-read-only-highlight-sparrow
Run status status check passed Passed #16283
Run duration 23m 17s
Commit git commit 74b5c9c18e: Remove console.logs
Committer eireland
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 1
Tests that did not run due to a developer annotating a test with .skip  Pending 4
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 149
View all changes introduced in this branch ↗︎

@codecov
Copy link

codecov bot commented Jul 31, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.20%. Comparing base (a8d909c) to head (74b5c9c).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2621      +/-   ##
==========================================
+ Coverage   86.18%   86.20%   +0.01%     
==========================================
  Files         782      782              
  Lines       42276    42272       -4     
  Branches    10787    10785       -2     
==========================================
+ Hits        36435    36439       +4     
+ Misses       5526     5518       -8     
  Partials      315      315              
Flag Coverage Δ
cypress ?
cypress-regression 78.29% <100.00%> (+0.03%) ⬆️
cypress-smoke 44.20% <16.66%> (+<0.01%) ⬆️
jest 48.45% <16.66%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@eireland eireland marked this pull request as ready for review July 31, 2025 12:14
@eireland eireland requested a review from bgoldowsky July 31, 2025 12:14
Copy link
Contributor

@bgoldowsky bgoldowsky left a comment

Choose a reason for hiding this comment

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

I do not recommend that we merge this PR without more work.
It fixes some cases but breaks others that used to work; for instance, sparrows no longer get repositioned properly as you are editing text in the text tile.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants