Skip to content

fix(browser): report dialog footer logo 18px, powered-by text 11px - #172

Merged
krassx merged 2 commits into
mainfrom
fix/report-dialog-logo-size
Oct 31, 2026
Merged

krassx merged 2 commits into
mainfrom
fix/report-dialog-logo-size

Conversation

@krassx

@krassx krassx commented Oct 30, 2026

Copy link
Copy Markdown
Contributor
  • .bugsee-reporting-dialog-logo: height: 30px → 18px
  • .bugsee-reporting-dialog-powered-text: font-size: 16px → 11px

A test pins both values. No other rule changed (the version line stays 9px).

🤖 Generated with Claude Code

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@claude

claude Bot commented Oct 30, 2026 •

Copy link
Copy Markdown

Claude finished @krassx's task in 18s —— View job


Review of PR #172: report dialog footer sizing

  • Read the diff, the surrounding CSS, and the existing style tests
  • Check the class prefix and selector assumptions in the new test
  • Post the review

No real issues found, so I left no inline comments.

What I checked

  • report-dialog-styles.ts:245-246 changes only the .logo height (30px → 18px) and the .powered-text font-size (16px → 11px). The .version rule (9px, opacity 0.3) and the grid layout are untouched, as the PR describes.
  • The logo is an inline SVG with viewBox="0 0 495 252" (report-dialog.ts:631-634). width: auto with a fixed height is how the aspect ratio is kept, so the 18px logo scales proportionally to about 35px wide.
  • The new test (report-dialog.test.ts:718-724) builds its selector from the literal bugsee-reporting-dialog-. That matches REPORT_DIALOG_CLASS_PREFIX, which already ends in - (report-dialog-styles.ts:21). The regex \{([^}]*)\} stops at the first }, which is safe here because both rules are single-line with no nested braces.
  • The ?? '' fallback means a renamed or removed rule yields an empty string, and the toMatch assertions then fail. The test can't pass by accident.
  • The not.toMatch(/height: 30px/) assertion is redundant next to the positive 18px match, but it's harmless.
  • Nothing else references the old 30px or 16px values. A search of packages and docs found no other users of them.
  • The test follows the style of the neighbouring version line and toolbar buttons tests, which check CSS text the same way.

Observations (not blocking)

  • .powered still sets font-size: 16px (line 244). With display: contents it has no box, and .powered-text now sets its own size, so the declaration does nothing. It could be removed or aligned for tidiness.
  • I did not run the tests or lint, and I did not view the dialog in a browser. The visual result at 18px and 11px, including how it looks on a two-row grid with the 9px version line, is unchecked. From the CSS the layout should hold: 11px text plus 9px version line is taller than the 18px logo, and align-items: center centres the logo across both rows.

Summary

  1. Overall risk: Low. This is a CSS-only change to two declarations, pinned by a test.
  2. Merge recommendation: Approve / merge once CI is green.
  3. Most important issues to fix: none.
  4. Positives: the change is small and matches the description. The test pins both values and guards against a silent selector mismatch.
    · branch fix/report-dialog-logo-size

@coderabbitai

coderabbitai Bot commented Oct 30, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Warning

Review limit reached

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

Next included review available in 49 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 11e51477-7d5c-4a0b-897f-de17251918c1
Commits

Reviewing files that changed from the base of the PR and between 9f2bd77 and fe07aa9.

Files selected for processing: 1
  • packages/browser/src/report-dialog.test.ts

No actionable comments were generated in the recent review.

Recent review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: f1278b23-029d-4ee0-810e-435ea4240627
Commits

Reviewing files that changed from the base of the PR and between 063d040 and 9f2bd77.

Files selected for processing: 2
  • packages/browser/src/report-dialog-styles.ts
  • packages/browser/src/report-dialog.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📝 Summary

Summary by CodeRabbit

  • Style
    • Reduced the powered-by footer logo height and text size in the report dialog.

The report dialog footer logo height changes from 30px to 18px. The “Powered by” text font size changes from 16px to 11px. A test checks the updated dimensions.

Suggested labels: javascript

Priority: Low

Change: Bug fix

Merge Risk: Minimal · previous review 9f2bd

The footer sizing matches the requested dimensions, with targeted regression assertions. No merge-blocking issue was identified.

Pre-merge checks: Passed 5
Check name Status Explanation
Title check Passed The title clearly and concisely identifies the browser report dialog footer logo and powered-by text size changes.
Description check Passed The description accurately explains both style changes, the test coverage, and the unchanged version text styling.
Docstring Coverage Passed No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 2 files.

Finishing Touches: 💡 1
🛠️ Fix failing CI checks: 💡
  • Commit to this branch
  • Create a new PR
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

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

@coderabbitai coderabbitai Bot added the javascript Pull requests that update javascript code label Oct 30, 2026
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@claude

claude Bot commented Oct 30, 2026 •

Copy link
Copy Markdown

Claude finished @krassx's task in 15s —— View job


Re-review of PR #172 after the latest push

  • Read the new commits and the full diff against origin/main
  • Compare against my earlier review
  • Post the update

No new findings, so I left no inline comments.

Since the last review

  • The only new commit is fe07aa94, a formatting change to the sizing test in report-dialog.test.ts:718-724. It wraps the ?? '' fallback onto its own line and changes no behavior.
  • The CSS change is the same as before: .logo height 30px → 18px and .powered-text font-size 16px → 11px in report-dialog-styles.ts:245-246.
  • REPORT_DIALOG_CSS is already imported in the test file (line 16), and the class prefix bugsee-reporting-dialog- matches REPORT_DIALOG_CLASS_PREFIX (report-dialog-styles.ts:21).
  • The test uses the same regex style as the neighbouring version rule test (report-dialog.test.ts:1221).

Earlier observation, still open and not blocking

  • .powered still sets font-size: 16px (report-dialog-styles.ts:244). It has no effect because the element is display: contents and .powered-text sets its own size. You could remove it or align it, but nothing breaks if you leave it.

Not verified

  • I did not run the tests, lint, or typecheck.
  • I did not view the dialog in a browser.

Summary

  1. Overall risk: Low. The change is CSS-only, two declarations, and a test pins both values.
  2. Merge recommendation: Approve / merge once CI is green.
  3. Issues to fix: none.
  4. Positives: the change is small and matches the description. The test fails if the rule is renamed or removed, because the empty fallback won't match.
    · branch fix/report-dialog-logo-size

@krassx
krassx merged commit 17b1acf into main Oct 31, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

javascript Pull requests that update javascript code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant