brooktrails/pr-reviewer
2
0
Fork 0
Code Issues 2 Pull requests Projects Releases Packages Wiki Activity Actions

feat: surface app version in dashboard header and group roles on hover #48

Merged
rcsheets merged 1 commit from feat/dashboard-version-header into main 2026-05-01 09:27:23 +00:00
Conversation 1 Commits 1 Files changed 4 +36 -15
rcsheets commented 2026-05-01 09:25:07 +00:00
Owner
Copy link

Two related header tweaks:

  • pageData and authErrorData both gain an AppVersion field, populated
    from server config. The layout renders it as a small muted "v0.19.2"
    next to the "pr-reviewer" title — same number that's stamped into
    every PR comment header, so operators have one less source of
    ambiguity about which build they're staring at.
  • pageData also picks up the session's Groups slice. The layout uses
    it to attach a tooltip to the username ("Roles: admins, viewers"),
    marking the span with a dotted underline + help cursor so it's
    clear the username is hoverable. Skipped silently when the session
    has no groups (e.g. a user not in any tracked group).

A small commaJoin template helper does the joining; renderAuthError
now backfills AppVersion from server config so callers don't have to
remember to set it on each authErrorData construction.

Co-Authored-By: Claude Opus 4.7 (1M context) noreply@anthropic.com

Two related header tweaks: - pageData and authErrorData both gain an AppVersion field, populated from server config. The layout renders it as a small muted "v0.19.2" next to the "pr-reviewer" title — same number that's stamped into every PR comment header, so operators have one less source of ambiguity about which build they're staring at. - pageData also picks up the session's Groups slice. The layout uses it to attach a tooltip to the username ("Roles: admins, viewers"), marking the span with a dotted underline + help cursor so it's clear the username is hoverable. Skipped silently when the session has no groups (e.g. a user not in any tracked group). A small commaJoin template helper does the joining; renderAuthError now backfills AppVersion from server config so callers don't have to remember to set it on each authErrorData construction. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
feat: surface app version in dashboard header and group roles on hover
All checks were successful
ci / check (pull_request) Successful in 42s
Details
c2989d2c73 
Two related header tweaks:

- pageData and authErrorData both gain an AppVersion field, populated
  from server config. The layout renders it as a small muted "v0.19.2"
  next to the "pr-reviewer" title — same number that's stamped into
  every PR comment header, so operators have one less source of
  ambiguity about which build they're staring at.
- pageData also picks up the session's Groups slice. The layout uses
  it to attach a tooltip to the username ("Roles: admins, viewers"),
  marking the span with a dotted underline + help cursor so it's
  clear the username is hoverable. Skipped silently when the session
  has no groups (e.g. a user not in any tracked group).

A small commaJoin template helper does the joining; renderAuthError
now backfills AppVersion from server config so callers don't have to
remember to set it on each authErrorData construction.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
pr-reviewer-bot commented 2026-05-01 09:25:08 +00:00
Collaborator
Copy link

Automated review by pr-reviewer v0.19.1 | Full Review | Claude Sonnet 4 | tracking id r-f47174-31167b
This is an AI-generated review and may contain mistkaes.

Status: ✅ Completed

✅ Verdict: Approve — Clean UI enhancement that adds helpful version visibility and role information with good error handling.
Complexity: trivial

Code Review: Surface app version and group roles in dashboard header

Summary

This PR enhances the dashboard header with two quality-of-life improvements: displaying the application version and showing user roles on hover. The implementation is clean and follows existing patterns.

What's Good

✅ Consistent version display: The version now appears in both PR comment headers and the dashboard, eliminating operator confusion about which build they're viewing.

✅ Thoughtful UX design: The role tooltip uses standard web conventions (dotted underline, help cursor) to indicate interactivity.

✅ Robust error handling: The renderAuthError function automatically backfills AppVersion from server config, preventing template errors if callers forget to set it.

✅ Template safety: The layout template gracefully handles missing data with conditional rendering ({{if .AppVersion}}, {{if .Groups}}).

✅ Code organization: The commaJoin helper is appropriately placed in the template function map and keeps the template readable.

Technical Implementation

The changes are well-structured:

  • Data flow: pageData and authErrorData structs gain AppVersion and Groups fields
  • Template helper: commaJoin function added to handle role list formatting
  • CSS styling: Version text is properly muted and sized; tooltip styling uses semantic cursor and border styles
  • Backwards compatibility: All new fields are optional in templates

Minor Observations

  • The VERSION file shows 0.15.0 while the PR description mentions v0.19.2 - this is expected since the VERSION file represents the current state, not the example in the description.
  • The Groups field in authErrorData is documented as "always nil here" which is correct since auth error pages don't have authenticated sessions.

Verdict

This is a clean, low-risk enhancement that improves operator experience without changing any core functionality. The implementation follows established patterns and includes appropriate safety measures.

📚 Context used in this review

Project guidance files:

  • CLAUDE.md

Repository fragments:

  • internal/config/config.go — extracted in answer to: "the definition and field type of AppVersion in the configuration struct"
  • VERSION (whole file)
*Automated review by [pr-reviewer](https://git.brooktrails.org/brooktrails/pr-reviewer) v0.19.1 | Full Review | Claude Sonnet 4 | tracking id `r-f47174-31167b`* *This is an AI-generated review and may contain mistkaes.* **Status:** ✅ Completed --- **✅ Verdict: Approve** — Clean UI enhancement that adds helpful version visibility and role information with good error handling. **Complexity:** trivial # Code Review: Surface app version and group roles in dashboard header ## Summary This PR enhances the dashboard header with two quality-of-life improvements: displaying the application version and showing user roles on hover. The implementation is clean and follows existing patterns. ## What's Good ✅ **Consistent version display**: The version now appears in both PR comment headers and the dashboard, eliminating operator confusion about which build they're viewing. ✅ **Thoughtful UX design**: The role tooltip uses standard web conventions (dotted underline, help cursor) to indicate interactivity. ✅ **Robust error handling**: The `renderAuthError` function automatically backfills `AppVersion` from server config, preventing template errors if callers forget to set it. ✅ **Template safety**: The layout template gracefully handles missing data with conditional rendering (`{{if .AppVersion}}`, `{{if .Groups}}`). ✅ **Code organization**: The `commaJoin` helper is appropriately placed in the template function map and keeps the template readable. ## Technical Implementation The changes are well-structured: - **Data flow**: `pageData` and `authErrorData` structs gain `AppVersion` and `Groups` fields - **Template helper**: `commaJoin` function added to handle role list formatting - **CSS styling**: Version text is properly muted and sized; tooltip styling uses semantic cursor and border styles - **Backwards compatibility**: All new fields are optional in templates ## Minor Observations - The VERSION file shows `0.15.0` while the PR description mentions `v0.19.2` - this is expected since the VERSION file represents the current state, not the example in the description. - The `Groups` field in `authErrorData` is documented as "always nil here" which is correct since auth error pages don't have authenticated sessions. ## Verdict This is a clean, low-risk enhancement that improves operator experience without changing any core functionality. The implementation follows established patterns and includes appropriate safety measures. <details> <summary>📚 Context used in this review</summary> **Project guidance files:** - `CLAUDE.md` **Repository fragments:** - `internal/config/config.go` — extracted in answer to: "the definition and field type of AppVersion in the configuration struct" - `VERSION` (whole file) </details>
rcsheets deleted branch feat/dashboard-version-header 2026-05-01 09:27:23 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
pr-reviewer-bot
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
brooktrails/pr-reviewer!48
No description provided.