👉 Workflow runs 👉 Branch on GitHub

Questions to Daniel on the design:

• how do we go back to the patch page from a review view? the global back button might not be enough if we have any navigation or state within the review page.
• why would we show labels there? would it make more sense to put the reviewer’s alias there?
• what are the contributors? folks who commented on the same review?
• do we still show the same second column, i.e. the patch list?
• should the main action be save/publish perhaps?
• should we maybe also show the patch title somewhere?

Screenshot 2025-02-10 at 17.55.09.png

• you can click the revision ID+description in the main content breadcrumbs to go back to the revision view (on the patch page)
• reviews can have labels AFAIK, don’t know if they are in any relation to the patch labels
• contributors are whoever participated in the review, creator by default plus anyone else who comments or reacts
• patch list should remain unchanged, idea being that we’re just navigating the main content area one level deeper, you’re still on the patch page - this is questionable, I think I explored ways to go 1 level deeper in the sidebar too, that would be: list of revisions, or list or reviews on a revision. lmk if you think we should further explore those
• main action should be discard/save/publish if it’s yours, and comment if it’s someone else’s
• patch title is still visible in the sidebar, that was one advantage of not changing that when navigating to a review page

Flesh out review page.

Change visuals.

Populate review view

Bring back the labels

Another pass on the overall design.

Fixup

Fix badges

Rebase.

Pull in newer heartwood

Need this for the public #labels method on reviews.

Style verdict dropdown hover states

Disallow empty summary on verdict none

Also fix saving empty summary.

Squash

Fix teaser overflow problems

Review of review page by rudolfs

Ok so this is my first review being written directly in the app, so:

• I don’t think we need the test feature flag here.
• A nit but could we keep up the full path imports here and here.
• We probably want allowedToEdit on the LabelInput and verdict changes only be true for the author of the review or if they are delegates of the repo, since else it should fail according to heartwood authorization rules.
• This if branch seems a bit off to me, couldn’t we do {@render children?.()}
• Could we use classes here as we do here I think that is nicer to read or to normalize the approach we want to take.

The rest of the patch lgtm so far. Nice work! Was able to write this review in the desktop app 🎉🥰 Used links to the explorer for code diffs.

Address first part of review comments

• ✅ I don’t think we need the test feature flag here.
• ✅ A nit but could we keep up the full path imports here and here.
• ✅ This if branch seems a bit off to me, couldn’t we do {@render children?.()}
• ✅ Could we use classes here as we do here I think that is nicer to read or to normalize the approach we want to take.

Only allow changing labels or state to author and delegates