Skip to content

MM-69631: add checked/unchecked indicator chip checklist items - #2307

Open
calebroseland wants to merge 5 commits into
masterfrom
feat/task-checked-timestamp-tooltip
Open

MM-69631: add checked/unchecked indicator chip checklist items#2307
calebroseland wants to merge 5 commits into
masterfrom
feat/task-checked-timestamp-tooltip

Conversation

@calebroseland

@calebroseland calebroseland commented Jun 17, 2026

Copy link
Copy Markdown
Member

Summary

Adds a chip beneath each checked or unchecked checklist task in the run's right-hand sidebar, showing when the state changed and who did it — information previously only visible in the run's activity/event list.

  • Compact chip (RHS row): an icon + relative time, e.g. ✓ 5d ago (checked), or an empty-checkbox icon for unchecked.
  • Tooltip: activity-log-style phrasing with the full fixed timestamp — <user> checked off <timestamp> / <user> unchecked <timestamp>.
  • Avatar: the acting user's avatar, when it can be resolved from the run's timeline events.
  • A compact prop toggles the short chip vs a Checked <relative> label (parallels the existing Due … chip).
  • Reorders the task metadata row to [checked] [assignee] [due date] [command].
CleanShot 2026-06-17 at 17 29 20

Implementation notes

  • Webapp-only. state_modified and timeline events already reach the client; no server / GraphQL / schema changes.
  • "Who" is resolved from timeline events passed down as props from the run (which holds them via useRun). Reading the Redux store directly is unreliable: the single-run fetch keeps the run in component-local state and does not hydrate the store.
  • The timeline-event ↔ item link has no stable id (events carry only the action + markdown-stripped title), so attribution matches on event_at === state_modified, then a unique-title tiebreak, and degrades to no-avatar on any ambiguity (e.g. multiple items changed in the same millisecond) rather than mis-attributing.

Testing

  • 30 unit tests (checked_chip.test.tsx + existing checklist_item suite): match / tiebreak / fallback, check + uncheck, compact / non-compact, icons.
  • Cypress E2E spec added (rdp_checked_chip_spec.js): check → chip + tooltip → uncheck → tooltip, plus no-chip-on-untouched-task.

Ticket Link

https://mattermost.atlassian.net/browse/MM-69631

Checklist

  • Gated by experimental feature flag (N/A)
  • Unit tests updated

Surface task completion in the run RHS: a compact chip under each checked
or unchecked checklist item shows when it changed ("checked 5d ago") and,
when resolvable from the run timeline events, the avatar of who did it. The
tooltip mirrors the activity log ("<user> checked off <timestamp>" /
"<user> unchecked <timestamp>") with the full fixed timestamp, degrading to
no attribution when the actor can't be matched unambiguously (e.g. multiple
items changed in the same millisecond). A compact prop toggles the short
chip vs the "Checked <relative>" form, paralleling the Due chip. Reorders
the task metadata row to [checked] [assignee] [due date] [command].

Timeline events are passed down as props from the run (which holds them via
useRun); reading the Redux store directly is unreliable because the single-
run fetch keeps the run in local state and does not hydrate the store.

Webapp-only: state_modified and timeline events already reach the client.

Ticket: https://mattermost.atlassian.net/browse/MM-69356
@calebroseland calebroseland added 1: UX Review Requires review by a UX Designer 2: Dev Review Requires review by a core committer labels Jun 17, 2026
@calebroseland calebroseland changed the title feat(checklist): add checked/unchecked chip to run task list MM-69356: add checked/unchecked indicator chip checklist items Jun 17, 2026
@coderabbitai

coderabbitai Bot commented Jun 17, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

This PR adds a CheckedChip component that displays who checked or unchecked a checklist item along with the relative/absolute time of the change, resolved from playbook run timeline events. It is wired into ChecklistItem and DraggableChecklistItem, with new i18n strings, unit tests, and an E2E spec.

Changes

Checked/Unchecked Chip Feature

Layer / File(s) Summary
CheckedChip component: props, event resolver, rendering, and styles
webapp/src/components/checklist_item/checked_chip.tsx, webapp/i18n/en.json
Adds CheckedChip with a Props interface, a memoized useStateEvent resolver matching timeline events to checklist items by exact timestamp and title disambiguation, parseDetails for safe JSON parsing, tooltip-based rendering with relative/absolute timestamps and Profile attribution, a styled Chip wrapper, and five new i18n translation keys.
ChecklistItem and DraggableChecklistItem wiring
webapp/src/components/checklist_item/checklist_item.tsx, webapp/src/components/checklist_item/checklist_item_draggable.tsx
Adds optional timelineEvents prop, showCheckedChip/renderCheckedChip helpers, updates row visibility logic, repositions the chip in row content, and forwards playbookRun?.timeline_events from the draggable wrapper.
CheckedChip unit tests and ChecklistItem mock
webapp/src/components/checklist_item/checked_chip.test.tsx, webapp/src/components/checklist_item/checklist_item.test.tsx
Adds mocks and factories for testing CheckedChip, covering null rendering, checked/unchecked states, attribution disambiguation, compact-mode icons, and adds a no-op mock of checked_chip for existing ChecklistItem tests.
Cypress E2E spec for RHS checked/unchecked chip
e2e-tests/cypress/tests/integration/playbooks/runs/rdp_checked_chip_spec.js
Adds an E2E spec creating a DM/run via API, verifying chip visibility and tooltip text transitions across check/uncheck actions, and chip absence for untouched tasks.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant DraggableChecklistItem
  participant ChecklistItem
  participant CheckedChip
  participant useStateEvent
  participant Profile

  DraggableChecklistItem->>ChecklistItem: timelineEvents (from playbookRun)
  ChecklistItem->>ChecklistItem: showCheckedChip(item)
  ChecklistItem->>CheckedChip: item, timelineEvents, compact
  CheckedChip->>useStateEvent: item, timelineEvents, action
  useStateEvent-->>CheckedChip: matched event or undefined
  CheckedChip->>Profile: subject userId (if resolved)
  CheckedChip-->>ChecklistItem: rendered chip or null
Loading

Suggested labels: QA Review Done

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly describes the main change: adding a checked/unchecked indicator chip for checklist items.
Description check ✅ Passed The description matches the implemented checklist chip, tooltip, avatar attribution, tests, and reordered row layout.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/task-checked-timestamp-tooltip

Comment @coderabbitai help to get the list of available commands.

@calebroseland

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jun 18, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@calebroseland

Copy link
Copy Markdown
Member Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Jun 18, 2026

Copy link
Copy Markdown
✅ Action performed

Full review finished.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@webapp/src/components/checklist_item/checked_chip.test.tsx`:
- Around line 147-152: The test 'falls back to a unique title match when no
event_at matches' in the checked_chip.test.tsx file currently expects an avatar
match based on title alone when timestamps do not match, which can cause false
actor attribution across tasks with similar titles. Change the assertion from
expecting ['carol'] to expecting an empty array, since attribution should only
succeed with a timestamp match. Title matching should only serve as a
tie-breaker when timestamps match exactly in the same millisecond, not as a
standalone fallback.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 907c366a-d0ef-47d0-bab6-8c73ca705bc6

📥 Commits

Reviewing files that changed from the base of the PR and between 30c1de8 and 23c0bef.

📒 Files selected for processing (7)
  • e2e-tests/tests/integration/playbooks/runs/rdp_checked_chip_spec.js
  • webapp/i18n/en.json
  • webapp/src/components/checklist_item/checked_chip.test.tsx
  • webapp/src/components/checklist_item/checked_chip.tsx
  • webapp/src/components/checklist_item/checklist_item.test.tsx
  • webapp/src/components/checklist_item/checklist_item.tsx
  • webapp/src/components/checklist_item/checklist_item_draggable.tsx

Comment thread webapp/src/components/checklist_item/checked_chip.test.tsx Outdated
Drop the title-only fallback used when no timeline event's event_at matched
the item's state_modified. The event_at === state_modified equality is the
authoritative link (the backend sets both from one clock read); title is now
only a tiebreaker among same-millisecond exact matches. Matching on title
across a different timestamp risked attributing the wrong user.

Addresses PR #2307 review feedback.
@calebroseland

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jun 18, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@calebroseland
calebroseland requested a review from JulienTant June 30, 2026 23:51

@JulienTant JulienTant 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.

The only blocker I can see is the wrong location of the e2e file. Brilliant idea to use the timeline!

@@ -0,0 +1,111 @@
// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.

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.

This file must be moved into the new cypress directory

// * Hovering the chip reveals the activity-style tooltip with the verb "checked off".
// Tooltip renders in a body-level portal, so assert at document scope.
cy.get('@taskRow').findByTestId('checklist-item-checked-chip').realHover();
cy.contains(/checked off/i).should('be.visible');

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.

should we use cy.uiGetToolTip rather than a whole body scan?

Comment thread webapp/src/components/checklist_item/checklist_item.tsx
Merging master removed src/components/widgets/tooltip in favor of the shared
WithTooltip component and moved e2e specs under cypress/. Update the
checked/unchecked chip to match:
- use WithTooltip (title prop) from @mattermost/shared/components/tooltip
- fix import ordering (the shared tooltip is now an external import)
- move the e2e spec to e2e-tests/cypress/tests/... and assert via cy.uiGetToolTip

Addresses PR #2307 review feedback and CI lint.
@calebroseland
calebroseland requested a review from JulienTant July 7, 2026 15:46
@calebroseland calebroseland changed the title MM-69356: add checked/unchecked indicator chip checklist items MM-69631: add checked/unchecked indicator chip checklist items Jul 7, 2026
@calebroseland
calebroseland requested a review from lindy65 July 7, 2026 15:50
@calebroseland calebroseland added the 3: QA Review Requires review by a QA tester label Jul 7, 2026
@lindy65 lindy65 added Setup Cloud Test Server Setup a test server using Mattermost Cloud and removed 1: UX Review Requires review by a UX Designer labels Jul 8, 2026
@mm-cloud-bot mm-cloud-bot removed the Setup Cloud Test Server Setup a test server using Mattermost Cloud label Jul 8, 2026
@mm-cloud-bot

Copy link
Copy Markdown

Test server destroyed

@lindy65

lindy65 commented Jul 8, 2026

Copy link
Copy Markdown

Hey @calebroseland - I couldn't get a spinwick spun up on the PR so I tested on a /cloud server running master and the artifact from this PR. Do you know why the spinwick didn't work?

Added my test plan to the Jira ticket and everything's looking good apart from my 2 questions below:

  1. I'm seeing on a task that I've skipped then restored that the tooltip wording on the restored task says "Unchecked [date] [time]" - should it not say "Restored [date] [time]"? (I guess the same question when skipping a task - should the tooltip say "Skipped [date] [time]"?)
image
  1. Changing my Settings > Display > Clock Display setting to use a 24-hour clock, I still see the time on the tooltip displayed in 12-hour clock mode. Is this expected? Intended?
image

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

Labels

3: QA Review Requires review by a QA tester

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants