Skip to content

refactor: pass the message directly to resolveJumpAnchor - #7675

Merged
diegolmello merged 1 commit into
native-34-roomview-hooksfrom
diegolmello/simpler-finding-6
Sep 11, 2026
Merged

refactor: pass the message directly to resolveJumpAnchor#7675
diegolmello merged 1 commit into
native-34-roomview-hooksfrom
diegolmello/simpler-finding-6

Conversation

@diegolmello

@diegolmello diegolmello commented Sep 11, 2026

Copy link
Copy Markdown
Member

Proposed changes

Pass the message-info result directly to resolveJumpAnchor instead of rebuilding a four-field copy of it at the call site. The result already carries every IJumpTarget field with identical types, so the copy translated nothing. IJumpTarget stays as the resolver's narrow contract and the resolver itself is untouched.

Removing the copy also closes a drift hazard: a field added to IJumpTarget later is now checked by the type system at the call site instead of silently arriving as undefined.

Issue(s)

Follow-up to #7482; targets its native-34-roomview-hooks branch.

How to test or reproduce

  • pnpm format-lint passed, including TypeScript compilation.
  • TZ=UTC pnpm test --runInBand --watchman=false passed: 315 suites, 2,902 tests, 426 snapshots.
  • Jump to a message from a reply, a mention and a search result, both inside and outside the current Message Window. Behavior is unchanged.

Screenshots

Not applicable; no visual changes.

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • Improvement (non-breaking change which improves a current function)
  • New feature (non-breaking change which adds functionality)
  • Documentation update (if none of the other choices apply)

Checklist

  • I have read the CONTRIBUTING doc
  • I have signed the CLA
  • Lint and unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works (if applicable)
  • I have added necessary documentation (if applicable)
  • Any dependent changes have been merged and published in downstream modules

Further comments

The two composed navigation tests now bind the message fixture to a constant and assert the resolver call exactly against it. The out-of-window case previously used a partial matcher, so the assertion is stricter than before. No tests were added or removed. Standards and specification reviews reported no findings.

Summary by CodeRabbit

  • Bug Fixes

    • Improved message navigation so jump anchors receive complete message details, supporting more accurate in-window and out-of-window navigation.
  • Tests

    • Strengthened navigation coverage by verifying exact message data is used when resolving jump anchors.

@coderabbitai

coderabbitai Bot commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: a96dfcf6-5c3e-4149-8fe4-256db4b76be7

📥 Commits

Reviewing files that changed from the base of the PR and between 9d2982a and 937b09a.

📒 Files selected for processing (2)
  • app/views/RoomView/hooks/__tests__/useRoomNavigation.composed.test.tsx
  • app/views/RoomView/hooks/useRoomNavigation.ts

Included review availability: Your plan provides up to 8 included reviews per hour; 5 remain after this review.

📜 Recent review details
⏰ Context from checks skipped due to timeout. (2)
  • GitHub Check: E2E Hold
  • GitHub Check: ESLint and Test / run-eslint-and-test
🧰 Additional context used
📓 Path-based instructions (3)
Format JavaScript and TypeScript code with Oxfmt using the repository configuration: tabs, single quotes, 130-character width, no trailing commas, omitted arrow-function parentheses where possible, and same-line brackets.

📄 CodeRabbit inference engine (CLAUDE.md)

Files:

  • app/views/RoomView/hooks/__tests__/useRoomNavigation.composed.test.tsx
  • app/views/RoomView/hooks/useRoomNavigation.ts
Use descriptive names for functions, variables, and classes that clearly convey their purpose Write comments that explain the 'why' behind code decisions, not the 'what' Keep functions small and focused on a single responsibility Use const...

📄 CodeRabbit inference engine (AGENTS.md)

Files:

  • app/views/RoomView/hooks/__tests__/useRoomNavigation.composed.test.tsx
  • app/views/RoomView/hooks/useRoomNavigation.ts
Use TypeScript for type safety; add explicit type annotations to function parameters and return types Prefer interfaces over type aliases for defining object shapes in TypeScript Use enums for sets of related constants rather than magic str...

📄 CodeRabbit inference engine (AGENTS.md)

Files:

  • app/views/RoomView/hooks/__tests__/useRoomNavigation.composed.test.tsx
  • app/views/RoomView/hooks/useRoomNavigation.ts
🔇 Additional comments (2)
app/views/RoomView/hooks/useRoomNavigation.ts (1)

148-148: LGTM!

app/views/RoomView/hooks/__tests__/useRoomNavigation.composed.test.tsx (1)

93-94: LGTM!

Also applies to: 104-104, 115-116, 124-124


Walkthrough

executeJump now passes the complete message object to resolveJumpAnchor. Composed tests reuse the lookup result and assert exact equality for in-window and out-of-window messages.

Changes

Room navigation jump data

Layer / File(s) Summary
Forward and verify complete message data
app/views/RoomView/hooks/useRoomNavigation.ts, app/views/RoomView/hooks/__tests__/useRoomNavigation.composed.test.tsx
executeJump passes the full message object to resolveJumpAnchor. Tests use a shared messageInfo value and verify exact resolver arguments for both message-location cases.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~5 minutes

Change: Refactor

Suggested labels: type: chore

Merge Risk: ⚪ Minimal · up to 937b0

The change preserves the resolver contract and is covered by passing tests.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: passing the full message directly to resolveJumpAnchor.
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.
  • Fix all pre-merge checks with AI

Warning

Errors were encountered while retrieving linked issues.

Errors (1)
  • JIRA integration encountered authorization issues. Please disconnect and reconnect the integration in the CodeRabbit UI.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@diegolmello
diegolmello merged commit eadf554 into native-34-roomview-hooks Sep 11, 2026
7 of 8 checks passed
@diegolmello
diegolmello deleted the diegolmello/simpler-finding-6 branch September 11, 2026 16:59
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.

1 participant