fix: make GraphicalDisplayRenderer subclassable

[forntoh]

Summary

Makes GraphicalDisplayRenderer properly subclassable by opening up the geometry helpers and active-item state that custom renderer subclasses need (reported against v5.13.0).

Changes

• src/renderer/GraphicalDisplayRenderer.h:
  • Moved measureText, toggleIndicatorWidth, rowHeight, drawScrollBar, getMaxRows, getMaxCols, getEffectiveCols from private to protected (Some GraphicalDisplayRenderer methods are private and should be at least protected #431).
  • Added public inline accessors getActiveItem() and isActiveItem(const MenuItem*) with Doxygen comments, exposing the previously private activeItem state (GraphicalDisplayRenderer activeItem information is not available to subclasses #432).
• Access-level change only — no behavior change. All other state members remain private.
• MenuRenderer::hasFocus was verified to already be protected, so no change was needed there.

Verification

• pio run -e esp32: SUCCESS (RAM 7.1%, flash 25.8%)
• pio run -e uno: compiles; fails only the pre-existing checkprogsize size limit, byte-identical to the pre-change baseline (local harness src/main.cpp includes all dependencies)
• Subclass compile proof: a minimal GraphicalDisplayRenderer subclass overriding drawItem and calling the newly protected members plus the new accessors compiles on both environments (proof code reverted afterward)
• clang-format --dry-run -Werror clean; git diff --check clean

Fixes #431
Fixes #432

Summary by CodeRabbit

• Improvements
  • Added access to the currently active menu item and a way to check whether a menu item is active.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Warning

Review limit reached

Next included review available in 17 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 2 included reviews currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 90bd391d-7986-442d-b41a-b27028266731

📥 Commits

Reviewing files that changed from the base of the PR and between d8d9723 and 379f699.

📒 Files selected for processing (1)

• src/renderer/GraphicalDisplayRenderer.h

📝 Walkthrough

Walkthrough

GraphicalDisplayRenderer adds getActiveItem() and isActiveItem() as public const accessors. The latter returns true only when the active item is non-null and matches the supplied item. A protected: access label is also added.

Priority: ⬇️ Low

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

Change: Feature · Severity of issue fixed: Low

Merge Risk: 🔵 Low · up to d8d97

A custom item callback can remove and delete itself while the renderer still exposes it as active, allowing a subclass to access freed storage. This is a narrow extension-code risk; clear the active context before removal to address it.

Security Architecture Review

Security architecture risk: 🔵 Low · up to d8d97

The new accessors do not change how items are selected or grant control over them. They do expose a borrowed item pointer whose validity during item removal is not guaranteed, so custom renderer code needs care when using it.

Retained concerns

• Low · security · inferred: The new public getter can expose a borrowed active-item pointer after reentrant removal and before lifecycle cleanup. If the owner deletes that item and custom code dereferences the getter result, freed storage could be accessed; no such runtime sequence is demonstrated.

Security review details

Security Blast Radius

• inferred — The plausible effect is confined to applications using the renderer API and custom item lifecycles; the reviewed change does not establish a cross-service or privileged sink path.

Security Findings and Attack Paths

• inferred — A custom item callback can run while its item is active and can reach screen removal. External deletion followed by a custom renderer’s dereference of the new getter would create a potential use-after-free path, but neither that subclass behavior nor attacker control of it is established.

Trust Boundaries and Controls

• observed — The accessors provide observation and identity comparison, not item mutation or callback registration. Active-item assignment remains on the existing renderer lifecycle path.

Hardening Proposals

• proposed — Specify the returned pointer’s borrowed lifetime for subclass authors and consider invalidating active-item state when removal can occur during a callback.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 1 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: enabling custom GraphicalDisplayRenderer subclasses through access-level updates and new accessors.
Linked Issues check	✅ Passed	The reviewed header places all seven methods named by #431 under protected: measureText, toggleIndicatorWidth, rowHeight, drawScrollBar, getMaxRows, getMaxCols, and getEffectiveCols. I…
Out of Scope Changes check	✅ Passed	The reviewed changes only alter GraphicalDisplayRenderer access and add active-item accessors. These changes directly support #431 and #432. No unrelated behavior, state exposure, or feature change …

✨ Finishing Touches 💡 2

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

🛠️ Fix failing CI checks 💡

• Commit to this branch
• Create a new PR

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

Won't fix