Skip to content

Add 'View full screen' compare links in diff comments #7

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ritikrishu merged 1 commit into from
Feb 21, 2026
Merged

Add 'View full screen' compare links in diff comments #7

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions dist/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ function buildDiffComment(data, commentId) {
'',
];
for (const file of data.files) {
lines.push(buildDiffFileSection(file));
lines.push(buildDiffFileSection(file, data.serviceUrl));
lines.push('');
}
// Add artifact download link
Expand Down Expand Up @@ -313,7 +313,7 @@ function buildDiffSummarySection(summary) {
/**
* Build section for a single file in diff mode.
*/
function buildDiffFileSection(file) {
function buildDiffFileSection(file, serviceUrl) {
const statusIcon = getStatusIcon(file.status);
const statusLabel = getStatusLabel(file.status);
const lines = [
Expand Down Expand Up @@ -354,7 +354,12 @@ function buildDiffFileSection(file) {
lines.push('#### ✏️ Modified Frames');
lines.push('');
for (const mod of diff.modified) {
lines.push(`**${escapeMarkdown(mod.frameName)}**`);
const compareUrl = serviceUrl && file.diff?.jobId
? `${serviceUrl}/compare/${file.diff.jobId}?frame=${encodeURIComponent(mod.frameName)}`
: null;
lines.push(compareUrl
? `**${escapeMarkdown(mod.frameName)}** · [View full screen](${compareUrl})`
: `**${escapeMarkdown(mod.frameName)}**`);
lines.push('');
lines.push('<table>');
lines.push('<tr><th>Before</th><th>After</th></tr>');
Expand Down Expand Up @@ -1269,6 +1274,7 @@ async function runDiffMode(inputs, octokit, prContext, changedFiles) {
prNumber: prContext.prNumber,
commitSha: prContext.headSha,
artifactUrl,
serviceUrl: inputs.serviceUrl,
};
const commentBody = (0, comment_builder_1.buildDiffComment)(commentData, inputs.commentId);
const postedCommentId = await (0, comments_1.postComment)(octokit, prContext.prNumber, commentBody, inputs.commentMode, inputs.commentId);
Expand Down Expand Up @@ -1699,6 +1705,7 @@ class ServiceRenderer extends base_1.BaseRenderer {
}
// Poll for completion (reuses existing polling logic)
const result = await this.pollForCompletion(submitData.jobId);
result.jobId = submitData.jobId;
if (result.errors && result.errors.length > 0) {
for (const err of result.errors) {
core.warning(`Diff error for frame "${err.frameName}" (${err.frameId}): ${err.error}`);
Expand Down
2 changes: 1 addition & 1 deletion dist/index.js.map
View file
Open in desktop

Large diffs are not rendered by default.

13 changes: 10 additions & 3 deletions src/comment-builder.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,7 @@ export function buildDiffComment(data: DiffCommentData, commentId?: string): str
];

for (const file of data.files) {
lines.push(buildDiffFileSection(file));
lines.push(buildDiffFileSection(file, data.serviceUrl));
lines.push('');
}

Expand Down Expand Up @@ -345,7 +345,7 @@ function buildDiffSummarySection(summary: DiffCommentSummary): string {
/**
* Build section for a single file in diff mode.
*/
function buildDiffFileSection(file: DiffFileCommentData): string {
function buildDiffFileSection(file: DiffFileCommentData, serviceUrl?: string): string {
const statusIcon = getStatusIcon(file.status);
const statusLabel = getStatusLabel(file.status);

Expand Down Expand Up @@ -393,7 +393,14 @@ function buildDiffFileSection(file: DiffFileCommentData): string {
lines.push('#### ✏️ Modified Frames');
lines.push('');
for (const mod of diff.modified) {
lines.push(`**${escapeMarkdown(mod.frameName)}**`);
const compareUrl = serviceUrl && file.diff?.jobId
? `${serviceUrl}/compare/${file.diff.jobId}?frame=${encodeURIComponent(mod.frameName)}`
: null;
lines.push(
compareUrl
? `**${escapeMarkdown(mod.frameName)}** · [View full screen](${compareUrl})`
: `**${escapeMarkdown(mod.frameName)}**`
);
Comment on lines +396 to +403
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Feb 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Normalize serviceUrl before URL construction; use diff.jobId instead of file.diff?.jobId.

Two issues:

  1. Trailing slash (minor): serviceUrl is the raw value from inputs.serviceUrl, passed through DiffCommentData without normalization. ServiceRenderer's constructor strips trailing slashes (service.ts:98), but that doesn't apply here. If a user configures serviceUrl as https://svc.example.com/, the generated link becomes https://svc.example.com//compare/... (double slash).

  2. Redundant optional chaining (nitpick): At this point the code is already inside if (file.diff) (line 377) with const diff = file.diff in scope. file.diff?.jobId should be diff.jobId.

🔧 Proposed fix 
-        const compareUrl = serviceUrl && file.diff?.jobId
-          ? `${serviceUrl}/compare/${file.diff.jobId}?frame=${encodeURIComponent(mod.frameName)}`
-          : null;
+        const baseUrl = serviceUrl?.replace(/\/+$/, '');
+        const compareUrl = baseUrl && diff.jobId
+          ? `${baseUrl}/compare/${diff.jobId}?frame=${encodeURIComponent(mod.frameName)}`
+          : null;
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/comment-builder.ts` around lines 396 - 403, The generated compare URL can
get a double slash and uses redundant optional chaining; normalize serviceUrl by
trimming any trailing slash before building compareUrl and use the
already-in-scope diff.jobId instead of file.diff?.jobId. Locate where compareUrl
is created (variable compareUrl in the block that references serviceUrl,
file.diff, mod.frameName and escapeMarkdown) and replace construction to use a
normalized serviceUrl (remove trailing '/' if present) and diff.jobId for the
path, keeping encodeURIComponent(mod.frameName) and existing
escapeMarkdown(mod.frameName) logic unchanged.

lines.push('');
lines.push('<table>');
lines.push('<tr><th>Before</th><th>After</th></tr>');
Expand Down
1 change: 1 addition & 0 deletions src/main.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,7 @@ async function runDiffMode(
prNumber: prContext.prNumber,
commitSha: prContext.headSha,
artifactUrl,
serviceUrl: inputs.serviceUrl,
};

const commentBody = buildDiffComment(commentData, inputs.commentId);
Expand Down
1 change: 1 addition & 0 deletions src/renderers/service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,7 @@ export class ServiceRenderer extends BaseRenderer {

// Poll for completion (reuses existing polling logic)
const result = await this.pollForCompletion(submitData.jobId) as DiffResponse;
result.jobId = submitData.jobId;

if (result.errors && result.errors.length > 0) {
for (const err of result.errors) {
Expand Down
2 changes: 2 additions & 0 deletions src/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@ export interface DiffSummary {

export interface DiffResponse {
type: 'diff';
jobId?: string;
success: boolean;
summary: DiffSummary;
added: DiffFrameScreenshot[];
Expand All @@ -185,6 +186,7 @@ export interface DiffCommentData {
prNumber: number;
commitSha: string;
artifactUrl?: string;
serviceUrl?: string;
}

export interface DiffCommentSummary {
Expand Down