P1B: Refactor (public/src/client/topic.js:157): Complex binary expression

[Sharkesh111]

Closes #236

Summary

Refactors handleBookmark in public/src/client/topic.js to remove the complex binary expression at line 157 by introducing well-named boolean variables:

• hasBookmark
• onFirstPageOrNoPagination
• exceedsBookmarkThreshold

This preserves behavior and improves readability/maintainability.

What this file does

public/src/client/topic.js initializes and handles client-side behavior on topic pages (e.g., scrolling to posts, bookmarks, code block handlers, previews, etc.). handleBookmark decides where to scroll on page load and whether to show bookmark instructions.

Scope of refactor

Only the conditional in handleBookmark governing the bookmark/instructions path (formerly at line 157). No logic changes elsewhere.

Qlty evidence

Before:
public/src/client/topic.js
157 Complex binary expression

After:
public/src/client/topic.js

✅ The “Complex binary expression” at line 157 is no longer reported.

Manual runtime validation (UI)

• I temporarily added console.log('P1B — VIVEK HANS — handleBookmark executed') inside the refactored block.
• Trigger steps:
  1. Start NodeBB and open any topic on page 1.
  2. In DevTools Console, set a bookmark for this topic:

     localStorage.setItem('topic:' + ajaxify.data.tid + ':bookmark', '2');

  3. Reload the page. When hasBookmark && onFirstPageOrNoPagination && exceedsBookmarkThreshold is true, the code path executes and the log appears.
• I took a screenshot of the Console showing this log and removed the temporary console.log before committing.

Lint & Test

• npm run lint has no new errors
• NODEBB_TEST_OFFLINE=1 DISABLE_PLUGIN_REGISTRY=1 npm run test → passes locally

Why this improves adaptability

The original compound condition obscured intent and made future changes risky. Naming sub-conditions documents the logic, makes diffs smaller, and isolates behavior changes to a single, readable line.

Alternatives considered

I considered extracting a helper shouldShowBookmarkAlert(...), but opted to keep the logic inline with named booleans to minimize surface area and avoid extra indirection in a hot path.

Risk/Impact

Low risk; refactor is a straight readability improvement with identical behavior.

Screenshot showing error on line 157 is gone now

[coveralls]

Pull Request Test Coverage Report for Build 17481130486

Details

• 0 of 0 changed or added relevant lines in 0 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage remained the same at 78.529%

---

Totals
Change from base Build 17435863312:	0.0%
Covered Lines:	24692
Relevant Lines:	29606

---

💛 - Coveralls