Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Nov 20, 2025

After completing a merge in Data Cleanup, protected senses show a warning icon with protection reasons, but protected entries do not. Also missing are audio and note summaries at the entry level.

Changes

  • Created ProtectedWarningIcon for general use
  • Refactored WordBox to use MUI Card components: Replaced nested Paper elements with CardHeader + CardContent structure matching DropWordCardHeader pattern
  • Added read-only summaries: ProtectedWarningIcon, AudioSummary, NoteButton, and FlagButton in header action
  • Fixed array key error in ReviewEntriesCompleted

Before:

<Paper>
  <Paper>
    <Stack>{vernacular}<FlagButton/></Stack>
  </Paper>
  {senses}
</Paper>

After:

<Card>
  <CardHeader 
    title={vernacular}
    action={<ProtectionIcon/><AudioSummary/><NoteButton/><FlagButton/>}
  />
  <CardContent>{senses}</CardContent>
</Card>

All protection data (reasons, audio count, note text) now visible in completed merge view, matching the pattern used during active merging.

Original prompt

This section details on the original issue you should resolve

<issue_title>Show protected entry protection reasons in completed merge</issue_title>
<issue_description>After using Merge Duplicates, you can go to Data Cleanup and, under "You've completed:", click on the tile with the completed merge. If the merge involved a protected sense, there will be a warning icon on the sense to so indicate. The hover text gives the protection reasons. However, the icon and reasons are missing for protected entries. Need to add that, as well as the read-only audio and note summaries, to the top of the entry.

Todo:

  • Refactor WordBox in src/goals/MergeDuplicates/MergeDupsCompleted.tsx into a WordCard:
    • Use MUI Card with both CardHeader and CardContent
    • The CardContent action should be like the action from DropWordCardHeader in src/goals/MergeDuplicates/MergeDupsStep/MergeDragDrop/DropWord.tsx, except with a FlagButton like what's in the WordBox</issue_description>

<agent_instructions>- commit messages satisfy .github/workflows/commit_message_check.yml

  • npm run fmt-frontend after any frontend changes
  • alphabetize component props</agent_instructions>

Comments on the Issue (you are @copilot in this section)


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.


This change is Reviewable

Copilot AI and others added 2 commits November 20, 2025 22:25
- Replace Paper-based WordBox with Card component using CardHeader and CardContent
- Add warning icon with tooltip for protected entries (similar to DropWordCardHeader)
- Display protection reasons in multiline tooltip
- Add AudioSummary, NoteButton, and FlagButton to header action
- Use lightyellow background for protected entries
- Import and use existing components from components/WordCard and MergeDuplicates/MergeDupsStep

Co-authored-by: imnasnainaec <[email protected]>
- Move tooltip text construction inside conditional block to avoid unnecessary computation
- Replace inline styles with sx prop for consistency with MUI patterns
- Remove unused theme import

Co-authored-by: imnasnainaec <[email protected]>
Copilot AI changed the title [WIP] Add protection reasons for completed merges Show protection indicators and reasons for entries in completed merges Nov 20, 2025
Copilot AI requested a review from imnasnainaec November 20, 2025 22:30
Copilot finished work on behalf of imnasnainaec November 20, 2025 22:31
@github-actions
Copy link

github-actions bot commented Nov 21, 2025

⚠️ Commit Message Format Issues ⚠️
commit bdc395f76b:
3: B1 Line exceeds max length (90>80): "- Move tooltip text construction inside conditional block to avoid unnecessary computation"

commit 65df42d5a0:
3: B1 Line exceeds max length (82>80): "- Replace Paper-based WordBox with Card component using CardHeader and CardContent"
4: B1 Line exceeds max length (85>80): "- Add warning icon with tooltip for protected entries (similar to DropWordCardHeader)"
8: B1 Line exceeds max length (95>80): "- Import and use existing components from components/WordCard and MergeDuplicates/MergeDupsStep"

@codecov
Copy link

codecov bot commented Nov 21, 2025

Codecov Report

❌ Patch coverage is 24.13793% with 22 lines in your changes missing coverage. Please review.
✅ Project coverage is 65.92%. Comparing base (f2888d6) to head (0c463ae).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
...eDuplicates/MergeDupsStep/ProtectedWarningIcon.tsx 0.00% 18 Missing ⚠️
src/goals/MergeDuplicates/MergeDupsCompleted.tsx 63.63% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4036      +/-   ##
==========================================
- Coverage   74.66%   65.92%   -8.74%     
==========================================
  Files         293      240      -53     
  Lines       10890     6134    -4756     
  Branches     1364      779     -585     
==========================================
- Hits         8131     4044    -4087     
+ Misses       2362     1839     -523     
+ Partials      397      251     -146     
Flag Coverage Δ
backend ?
frontend 65.92% <24.13%> (-0.19%) ⬇️

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.

Copy link
Collaborator

@imnasnainaec imnasnainaec left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 5 files reviewed, all discussions resolved (waiting on @copilot)

This comment was marked as resolved.

@imnasnainaec imnasnainaec marked this pull request as ready for review November 21, 2025 17:18
@imnasnainaec imnasnainaec added the 🟨Medium Medium-priority PR label Nov 24, 2025
Copy link
Contributor

@jasonleenaylor jasonleenaylor left a comment

Choose a reason for hiding this comment

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

:lgtm:

@jasonleenaylor reviewed 3 of 5 files at r2, 2 of 2 files at r3, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @copilot and @imnasnainaec)

@imnasnainaec imnasnainaec removed their request for review November 25, 2025 20:19
Copy link
Collaborator

@imnasnainaec imnasnainaec left a comment

Choose a reason for hiding this comment

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

@imnasnainaec reviewed 1 of 1 files at r4, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @copilot)

@imnasnainaec imnasnainaec merged commit 7808205 into master Nov 25, 2025
15 of 16 checks passed
@imnasnainaec imnasnainaec deleted the copilot/add-protection-reasons-to-merge branch November 25, 2025 20:24
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.

Show protected entry protection reasons in completed merge

3 participants