web: fix blob header seam, popup comment composers, split Reviews from Commits
commit
b0b1145web: fix blob header seam, popup comment composers, split Reviews from Commits
The raw-source blob view nested .blob-header inside .blob’s own
bordered box instead of rendering it as a sibling, breaking the
seamless rounded-header/rounded-body pairing every other view kind
already got right (markdown, asciidoc, binary) — header now always
precedes .blob`, never sits inside its padding.
"Comment on this file" and "comment on this commit" navigated to a
separate /comments page instead of opening in place. The file-level
composer template now renders on every blob view kind (not just raw
source — a whole-file comment needs no per-line gutter to make sense),
and a new ents.js handler toggles it as a floating popup. The
commit-level trigger needed real plumbing behind it: submitting the old
link’s empty path would have hit MissingPath on an anchor that can
never resolve, so it now posts through a new commits/<oid> comment
context (POST /commit/{oid}/comment), mirroring the existing
review-comment pattern, with its own popup and a merged Conversation
section.
The rail’s "Review" icon just linked to /commits, with no place to
see reviews across commits — added a Reviews tab (the sprite already
had an unused i-review icon anticipating this) and a /reviews
aggregate list, and renamed the old label back to Commits.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Reviews
No reviews of this commit yet — record a verdict below.
Start a review
crates/cli/ents-web/src/router.rs
@@ -69,10 +69,12 @@
.route("/commits", get(pages::commits::list::<O>))
.route("/commit/{oid}", get(pages::commits::show::<O>))
.route("/commit/{oid}/review", post(pages::commits::review::<O>))
+ .route("/commit/{oid}/comment", post(pages::commits::comment::<O>))
.route(
"/reviews/{target}/{member}/comment",
post(pages::commits::review_comment::<O>),
)
+ .route("/reviews", get(pages::reviews::list::<O>))
.route("/files", get(pages::files::root::<O>))
.route("/files/{*path}", get(pages::files::show::<O>))
.route("/meta", get(pages::meta::show::<O>))
crates/cli/ents-web/tests/router.rs
@@ -1092,6 +1092,7 @@
"/",
"/files",
"/commits",
+ "/reviews",
"/issues",
"/comments",
"/meta",
@@ -1102,7 +1103,7 @@
"the rail links {href}"
);
}
- for label in ["Dashboard", "Code", "Review", "Issues", "Threads"] {
+ for label in ["Dashboard", "Code", "Commits", "Reviews", "Issues", "Threads"] {
assert!(
overview.contains(&format!("title=\"{label}\"")),
"the rail tooltips {label}"
@@ -1741,12 +1742,13 @@
);
}
-/// A doc-rendered (Markdown) blob view carries no composer template at
-/// all: there is no source line row for `assets/ents.js` to anchor an
-/// inline composer after, so `ents_web::pages::files::blob_view` never
-/// renders one for this view kind.
+/// A doc-rendered (Markdown) blob view still carries the whole-file
+/// composer template: there is no per-line gutter row for `assets/ents.js`
+/// to open one against a specific line range, but "comment on this file"
+/// (the template's own empty `lines` default) is exactly as meaningful on
+/// a rendered document as on a raw-source view.
#[tokio::test]
-async fn files_markdown_blob_view_has_no_composer_template() {
+async fn files_markdown_blob_view_carries_the_composer_template() {
let dir = seed_repo(&[("docs/x.md", "# Doc Title\n\nSome text.\n")]);
let state = build_state_at(
FixtureIdentity {
@@ -1774,8 +1776,9 @@
.to_bytes();
let body = String::from_utf8(body.to_vec()).expect("utf8 html");
assert!(
- !body.contains("composer-template"),
- "a doc-rendered view has no source line to anchor a composer to"
+ body.contains("id=\"composer-template\""),
+ "a doc-rendered view has no per-line gutter, but \"comment on this \
+ file\" (empty `lines`) is exactly as meaningful there"
);
}
crates/cli/ents-web/src/assets/ents.css
@@ -608,6 +608,7 @@
* `crate::pages::files`: a blob header bar over the line-per-row source
* table. */
.blob-header {
+ position: relative;
display: flex; align-items: center; flex-wrap: wrap; gap: 8px 12px; padding: 11px 15px;
background: var(--surface); border: 1px solid var(--line); border-bottom: none;
border-radius: var(--radius-inner) var(--radius-inner) 0 0;
@@ -618,6 +619,11 @@
.blob-actions { margin-left: auto; display: flex; align-items: center; gap: 12px; font-size: 12.5px; color: var(--ink-3); }
.blob-actions a { color: var(--ink-2); }
.blob-actions a:hover { color: var(--accent); }
+/* A "comment on this file/commit" trigger's popup (`assets/ents.js`):
+ * floats below its trigger's own header row instead of pushing the
+ * header/content box's own seamless border pairing apart. */
+.standalone-composer { position: absolute; top: calc(100% + 8px); right: 14px; z-index: 20; width: min(480px, calc(100vw - 60px)); }
+.standalone-composer .composer-form { margin: 0; max-width: none; }
.blob {
overflow-x: auto; background: var(--surface); border: 1px solid var(--line);
border-radius: 0 0 var(--radius-inner) var(--radius-inner); margin-bottom: 18px; padding: 8px 0 10px;
@@ -675,7 +681,7 @@
* element rules (headings, lists, code spans) apply, scaled down to this
* compact metadata context instead of a full document card's. */
.commit-msg.doc-body { padding: 0; max-width: none; font-size: 13px; margin-bottom: 8px; }
-.commit-meta { display: flex; align-items: center; gap: 10px; flex-wrap: wrap; color: var(--ink-3); font-size: 12px; font-family: var(--mono); margin-top: 6px; }
+.commit-meta { position: relative; display: flex; align-items: center; gap: 10px; flex-wrap: wrap; color: var(--ink-3); font-size: 12px; font-family: var(--mono); margin-top: 6px; }
.commit-meta a { color: var(--ink-3); }
.commit-meta a:hover { color: var(--accent); }
crates/cli/ents-web/src/assets/ents.js
@@ -220,3 +220,45 @@
cell.appendChild(button);
});
})();
+
+/*
+ * Standalone comment triggers -- "comment on this file"
+ * (`crate::pages::files::blob_header`) and "comment on this commit"
+ * (`crate::pages::commits::commit_comment_template`) -- toggle a
+ * server-rendered `<template>` as a floating popup right under their own
+ * header/meta row, instead of navigating to a full add-comment page.
+ * Each trigger's `href` stays a real no-JS fallback.
+ */
+(function () {
+ "use strict";
+
+ document.querySelectorAll("a.composer-trigger[data-composer]").forEach(function (trigger) {
+ var template = document.getElementById(trigger.getAttribute("data-composer"));
+ var host = trigger.closest(".blob-header, .commit-meta");
+ if (!template || !host) {
+ return;
+ }
+ trigger.addEventListener("click", function (event) {
+ event.preventDefault();
+ var existing = host.querySelector(".standalone-composer");
+ if (existing) {
+ existing.remove();
+ return;
+ }
+ var wrapper = document.createElement("div");
+ wrapper.className = "standalone-composer";
+ wrapper.appendChild(template.content.cloneNode(true));
+ var cancel = wrapper.querySelector(".composer-cancel");
+ if (cancel) {
+ cancel.addEventListener("click", function () {
+ wrapper.remove();
+ });
+ }
+ host.appendChild(wrapper);
+ var textarea = wrapper.querySelector("textarea");
+ if (textarea) {
+ textarea.focus();
+ }
+ });
+ });
+})();
crates/cli/ents-web/src/pages/commits.rs
@@ -273,6 +273,13 @@
let checks = checks_section(&state, object_id);
let reviews = reviews_section(&state, &session, object_id, &oid);
let (sidebar_rows, _older) = commit_rows(&state, None, PAGE_SIZE);
+ let commit_context = format!("commits/{oid}");
+ let commit_thread = ents_forge::comment::thread(
+ state.refs.as_ref(),
+ &*state.objects(),
+ &commit_context,
+ )
+ .unwrap_or_default();
Ok(super::layout_split(
&super::RepoHeader::from_state(&state),
@@ -313,8 +320,10 @@
" \u{b7} root commit"
}
" \u{b7} "
- a href={ "/comments?rev=" (object_id) } { "comment on this commit" }
+ a.composer-trigger data-composer="commit-composer-template"
+ href={ "/comments?rev=" (object_id) } { "comment on this commit" }
}
+ (commit_comment_template(&oid, &session))
}
}
(checks)
@@ -324,12 +333,13 @@
@if truncated {
div.card { div.binary { "Diff truncated (over " (MAX_DIFF_BYTES / (1024 * 1024)) " MiB)." } }
}
- @if !comments.is_empty() {
+ @if !comments.is_empty() || !commit_thread.is_empty() {
div.readable {
h2 { "Conversation" }
@for (index, comment) in comments.iter().enumerate() {
(super::comments::comment_card(index, comment, super::comments::LinkMode::CrossFile))
}
+ (super::comments::thread_section(&state, &session, &commit_thread, &format!("/commit/{oid}")))
}
}
},
@@ -605,6 +615,95 @@
/// `model.review` makes it a hard enum, unlike issue and comment states --
/// defaulting to `approve`, the same default a bare `select`'s first option
/// would submit.
+/// The commit-level comment composer's own hidden `<template>`
+/// (`crate::pages::files::composer_template`'s counterpart for a commit,
+/// which has no file of its own to anchor a path-based comment to): posts
+/// to [`comment`], naming `commits/<oid>` as its context
+/// (`model.comment-context`) rather than anchoring to a path, exactly the
+/// way [`review_comment_form`] names `reviews/<target>/<member>`. Cloned
+/// by `assets/ents.js`'s standalone-composer trigger, opened from the
+/// "comment on this commit" link beside the parents list; with JS
+/// disabled that link remains a real navigation to `/comments?rev=`
+/// instead (a page-less no-JS fallback for this specific context would be
+/// its own added surface, so it stays the plain path-anchored form for
+/// now).
+fn commit_comment_template(oid: &str, session: &Session) -> Markup {
+ html! {
+ template id="commit-composer-template" {
+ form.composer-form method="post" action=(format!("/commit/{oid}/comment")) {
+ (super::csrf_input(session))
+ input type="hidden" name="return_to" value=(format!("/commit/{oid}"));
+ textarea name="body" placeholder="Leave a comment on this commit" {}
+ div.composer-buttons {
+ button type="submit" { "Comment" }
+ button.composer-cancel type="button" { "Cancel" }
+ }
+ }
+ }
+ }
+}
+
+/// The form fields `POST /commit/{oid}/comment` accepts.
+#[derive(Debug, Deserialize)]
+pub struct CommitCommentForm {
+ /// The comment's body text.
+ body: String,
+ /// The per-session CSRF token (`roots.web-session`).
+ csrf: String,
+ /// Where to send the browser back to; honored only when it is a
+ /// same-origin path.
+ #[serde(default)]
+ return_to: String,
+}
+
+/// `POST /commit/{oid}/comment`: a comment naming `commits/<oid>` as its
+/// context (`model.comment-context`) -- unanchored, exactly like
+/// [`review_comment`], since a comment about the commit as a whole has no
+/// path to anchor to the way a file-anchored comment does.
+///
+/// # Errors
+///
+/// [`Error::BadCsrf`] if `form.csrf` does not match; otherwise propagates
+/// [`ents_forge::comment::add`]'s own failures.
+// @relation(model.comment-context, roots.web-signing, roots.web-session, scope=function)
+pub async fn comment<O>(
+ State(state): State<Arc<AppState<O>>>,
+ axum::Extension(session): axum::Extension<Session>,
+ Path(oid): Path<String>,
+ Form(form): Form<CommitCommentForm>,
+) -> Result<impl IntoResponse>
+where
+ O: Find + Write + Send + 'static,
+{
+ super::require_csrf(&session, &form.csrf)?;
+ let identity = state.identity.as_ref();
+ let new = ents_forge::comment::NewComment {
+ body: form.body,
+ path: None,
+ lines: None,
+ rev: "HEAD".to_owned(),
+ worktree: false,
+ context: Some(format!("commits/{oid}")),
+ parent: None,
+ };
+ let (_comment_id, outcome) = ents_forge::comment::add(
+ state.refs.as_ref(),
+ &*state.objects(),
+ state.events.as_ref(),
+ &state.path,
+ new,
+ &crate::receive_identity!(identity, crate::pages::member_author(&session)),
+ state.mode,
+ )?;
+ crate::error::outcome_to_result(outcome)?;
+ let target = if form.return_to.starts_with('/') {
+ form.return_to
+ } else {
+ format!("/commit/{oid}")
+ };
+ Ok(Redirect::to(&target))
+}
+
fn start_review_form(session: &Session, oid: &str) -> Markup {
html! {
h3 { "Start a review" }
crates/cli/ents-web/src/pages/files.rs
@@ -523,11 +523,18 @@
editor: editor.clone(),
})
};
+ // Every view kind carries the whole-file composer template -- a
+ // doc-rendered or binary view has no per-line gutter to open one with
+ // a specific line range, but "comment on this file" (empty `lines`,
+ // [`composer_template`]'s own default) is exactly as meaningful there
+ // as on a raw-source view.
+ let composer = composer_template(path, head_oid, session);
if is_binary(bytes) {
return Ok((
html! {
(no_line_count_header())
div.binary { "Binary file (" (bytes.len()) " bytes) not shown." }
+ (composer)
},
super::comments::comments_section(comments),
));
@@ -537,6 +544,7 @@
html! {
(no_line_count_header())
div.binary { "Binary file (" (bytes.len()) " bytes) not shown." }
+ (composer)
},
super::comments::comments_section(comments),
));
@@ -546,6 +554,7 @@
html! {
(no_line_count_header())
div.card { div.doc-body { (crate::markdown::to_html(text)) } }
+ (composer)
},
super::comments::comments_section(comments),
));
@@ -555,6 +564,7 @@
html! {
(no_line_count_header())
div.card { div.doc-body { (crate::asciidoc::to_html(text)?) } }
+ (composer)
},
super::comments::comments_section(comments),
));
@@ -571,7 +581,6 @@
comments: comment_count,
editor,
});
- let composer = composer_template(path, head_oid, session);
let below: Vec<(usize, &super::comments::FileComment)> = comments
.iter()
.enumerate()
@@ -658,7 +667,7 @@
(meta.comments) @if meta.comments == 1 { " comment" } @else { " comments" }
}
}
- a href={ "/comments?file=" (meta.path) } { "comment on this file" }
+ a.composer-trigger data-composer="composer-template" href={ "/comments?file=" (meta.path) } { "comment on this file" }
}
}
}
@@ -758,8 +767,14 @@
}
html! {
+ // `header` is a sibling before `.blob`, never nested inside it --
+ // `.blob-header`'s own border and top-only radius are meant to sit
+ // flush atop `.blob`'s bottom-only radius as one continuous box
+ // (`ents.css`'s own note), which only holds when the two are
+ // siblings, not when the header sits inset inside `.blob`'s own
+ // padded, fully-bordered box.
+ (header)
div.blob data-path=(path) data-rev=(head_oid) {
- (header)
table {
tbody {
@for (index, code) in code_lines.into_iter().enumerate() {
@@ -1204,7 +1219,7 @@
}
#[test]
- fn blob_view_carries_the_composer_hooks_only_on_a_raw_source_view() {
+ fn blob_view_carries_the_composer_hooks_on_every_view_kind() {
let (body, _below) = blob_view(
"src/main.rs",
"main.rs",
@@ -1237,8 +1252,9 @@
)
.expect("markdown renders");
assert!(
- !doc_body.into_string().contains("composer-template"),
- "a doc-rendered view has no source line to anchor a composer to"
+ doc_body.into_string().contains("id=\"composer-template\""),
+ "a doc-rendered view has no per-line gutter, but \"comment on \
+ this file\" (empty `lines`) is exactly as meaningful there"
);
}
crates/cli/ents-web/src/pages/mod.rs
@@ -15,12 +15,16 @@
//! [`effects`], [`toolchains`], [`redactions`], and [`inbox`] additionally
//! share one `meta` rail item and `META_SECTIONS` rail rather than each
//! carrying its own top-level entry (see `Tab`'s own doc); [`meta`] is that
-//! group's `GET /meta` landing page. [`commits`] and [`issues`] are rail
-//! items of their own -- `Tab::Commits` (Review) and `Tab::Issues`
-//! (Issues) in [`layout`]'s icon rail, alongside the dashboard, code,
-//! threads, and meta items. [`search`] renders with no rail item active at
-//! all; it is reached from the `.wb-bar`'s own `.palette` search form
-//! rather than any rail item.
+//! group's `GET /meta` landing page. [`commits`], [`reviews`], and
+//! [`issues`] are rail items of their own -- `Tab::Commits`, `Tab::Reviews`,
+//! and `Tab::Issues` in [`layout`]'s icon rail, alongside the dashboard,
+//! code, threads, and meta items. `reviews::list` (`GET /reviews`) is a
+//! read-only aggregate across every commit's own reviews
+//! (`commits::reviews_section` renders the same [`ents_forge::review`]
+//! entities scoped to one commit; this module has no writes of its own --
+//! every mutation still posts through `commits`'s own routes). [`search`]
+//! renders with no rail item active at all; it is reached from the
+//! `.wb-bar`'s own `.palette` search form rather than any rail item.
pub mod account;
pub mod comments;
@@ -34,6 +38,7 @@
pub mod members;
pub mod meta;
pub mod redactions;
+pub mod reviews;
pub mod search;
pub mod toolchains;
@@ -111,20 +116,27 @@
/// (the pre-redo `Tab` enum, carried through the workbench restructure:
/// the horizontal tab strip became the vertical icon rail, but the
/// "handler names its own section" contract is unchanged). The rail reads,
-/// top to bottom: Dashboard (`Overview`), Code (`Files`), Review
-/// (`Commits`), Issues, Threads (`Comments`); then, past the
-/// spacer, Repo & governance (`Meta`) and Account. `Meta` covers five page
-/// families ([`super::members`], [`super::effects`], [`super::toolchains`],
+/// top to bottom: Dashboard (`Overview`), Code (`Files`), Commits, Reviews,
+/// Issues, Threads (`Comments`); then, past the spacer, Repo & governance
+/// (`Meta`) and Account. Commits and Reviews are two rail items, not one,
+/// even though every review still lives on its own commit's page
+/// (`super::commits::reviews_section`) -- browsing history and judging a
+/// specific commit are different reasons to be on this rail, so they get
+/// their own icons (`super::reviews` is the read-only aggregate list; no
+/// mutation route lives there). `Meta` covers five page families
+/// ([`super::members`], [`super::effects`], [`super::toolchains`],
/// [`super::redactions`], [`super::inbox`]) behind one rail item and the
/// [`META_SECTIONS`] rail (see [`layout_meta`]) rather than an item each --
-/// nine equal entries did not scale as page families grew. `None`
-/// highlights nothing at all, for a page that is not part of any rail
-/// item's own section ([`super::search`]'s results page).
+/// unrelated to the Commits/Reviews split above: those five are one page
+/// family each with no reason to be found separately, unlike Commits and
+/// Reviews. `None` highlights nothing at all, for a page that is not part
+/// of any rail item's own section ([`super::search`]'s results page).
#[derive(Clone, Copy, PartialEq, Eq)]
pub(crate) enum Tab {
Overview,
Files,
Commits,
+ Reviews,
Issues,
Comments,
Meta,
@@ -294,7 +306,8 @@
span.nav-mark { "ge" }
(rail_link(active, Tab::Overview, "/", "Dashboard", "i-home"))
(rail_link(active, Tab::Files, "/files", "Code", "i-files"))
- (rail_link(active, Tab::Commits, "/commits", "Review", "i-commit"))
+ (rail_link(active, Tab::Commits, "/commits", "Commits", "i-commit"))
+ (rail_link(active, Tab::Reviews, "/reviews", "Reviews", "i-review"))
(rail_link(active, Tab::Issues, "/issues", "Issues", "i-issue"))
(rail_link(active, Tab::Comments, "/comments", "Threads", "i-comment"))
span.spacer {}
crates/cli/ents-web/src/pages/reviews.rs
@@ -1,0 +1,98 @@
+//! `GET /reviews`: every review recorded in this repository, newest
+//! first -- a read-only aggregate across commits, alongside
+//! `crate::pages::commits`'s own per-commit `reviews_section` rather than
+//! replacing it. Every mutation (starting a review, commenting on one)
+//! still posts through `commits`'s own routes; this module has none of
+//! its own.
+
+use std::sync::Arc;
+
+use gix::bstr::ByteSlice as _;
+use gix_object::{Find, Write};
+use maud::{Markup, html};
+
+use crate::error::Result;
+use crate::state::AppState;
+
+/// `GET /reviews`: list [`ents_forge::review::list`]'s full result (no
+/// `target` filter), newest reviewer-commit first. Best effort per row,
+/// mirroring `crate::pages::commits::reviews_section`'s own stance: a
+/// review whose reviewer-commit chain or target subject fails to read
+/// still renders, just without the piece that failed, rather than
+/// dropping the row or failing the page.
+///
+/// # Errors
+///
+/// Propagates a ref-store or object read failure enumerating the reviews
+/// themselves ([`ents_forge::review::list`]); a per-row read failure
+/// degrades that row instead of failing the page.
+pub async fn list<O>(
+ axum::extract::State(state): axum::extract::State<Arc<AppState<O>>>,
+) -> Result<Markup>
+where
+ O: Find + Write + Send + 'static,
+{
+ let mut rows = ents_forge::review::list(state.refs.as_ref(), &*state.objects(), &state.path, None)
+ .unwrap_or_default();
+ let repo = gix::open(&state.path).ok();
+
+ let mut with_time: Vec<(i64, Markup)> = rows
+ .drain(..)
+ .map(|((target, member), review)| {
+ let reviewer = ents_model::namespace::review_ref(&target, &member)
+ .ok()
+ .and_then(|ref_name| state.refs.get(ref_name.as_ref()).ok().flatten())
+ .and_then(|tip| super::commit_authorship(&*state.objects(), tip).ok());
+ let seconds = reviewer.as_ref().map_or(0, |(_author, seconds)| *seconds);
+ let subject = repo.as_ref().and_then(|repo| {
+ let oid = gix_hash::ObjectId::from_hex(target.as_bytes()).ok()?;
+ let commit = repo.find_commit(oid).ok()?;
+ let message = commit.message().ok()?;
+ Some(message.title.to_str_lossy().into_owned())
+ });
+ let short = target.get(..7).unwrap_or(&target).to_owned();
+ (
+ seconds,
+ html! {
+ div.card {
+ div.comment-meta {
+ span class={ "verdict verdict-" (review.verdict) } { (review.verdict) }
+ (super::avatar(member.as_str()))
+ span.author { (member) }
+ span.spacer {}
+ a href={ "/commit/" (target) } { code { (short) } }
+ @if let Some(subject) = &subject {
+ span.muted { (subject) }
+ }
+ }
+ @if let Some((_author, seconds)) = &reviewer {
+ span.entry-size { (super::ago(*seconds)) }
+ }
+ }
+ },
+ )
+ })
+ .collect();
+ with_time.sort_by(|(a, _), (b, _)| b.cmp(a));
+
+ Ok(super::layout(
+ &super::RepoHeader::from_state(&state),
+ &super::identity_label(&state),
+ super::Tab::Reviews,
+ "Reviews",
+ html! {
+ div.readable {
+ @if with_time.is_empty() {
+ (super::blankslate(
+ "No reviews yet",
+ html! { "Record one from a commit's own page." },
+ ))
+ } @else {
+ @for (_seconds, card) in &with_time {
+ (card)
+ }
+ }
+ }
+ },
+ ))
+}