Skip to content

fix responsive issue in settings/api_key route page#1566

Open
francmart514 wants to merge 7 commits intowe-promise:mainfrom
francmart514:fix/api-key-responsive-md-range
Open

fix responsive issue in settings/api_key route page#1566
francmart514 wants to merge 7 commits intowe-promise:mainfrom
francmart514:fix/api-key-responsive-md-range

Conversation

@francmart514
Copy link
Copy Markdown

@francmart514 francmart514 commented Apr 27, 2026

Description

This PR fixes a responsive layout issue on Settings > API Key in the medium-width range (roughly 768px–987px) where the content pane did not shrink correctly when the settings sidebar remained visible, causing the right side to be clipped.

It also improves API key block resilience by preventing long key/curl content from forcing overflow and cleans up curl snippet formatting.

Closes #1565

Changes made:
Updated app/views/layouts/settings.html.erb to allow proper content shrinking in split-pane widths (min-w-0 on right-pane containers, header row wrapping, safer page action container sizing).
Refactored API key display sections in:
app/views/settings/api_keys/show.html.erb
app/views/settings/api_keys/created.html.erb
app/views/settings/api_keys/created.turbo_stream.erb to use a stable two-row key/copy layout for narrow/medium widths.
Hardened key and curl snippet containers against overflow with wrapping/scroll-safe classes.
Replaced inline full-key curl examples with X-Api-Key: <YOUR_API_KEY> placeholder to avoid layout blowout from long tokens.
Removed extra leading spacing in curl snippet blocks by dropping whitespace-pre-wrap from the curl container.
Updated system test expectation in test/system/settings/api_keys_test.rb to match the placeholder curl snippet.

Summary by CodeRabbit

  • Bug Fixes

    • Prevented horizontal overflow and ensured elements can shrink properly; copy confirmation feedback now reliably resets.
  • Style

    • Improved header wrapping, spacing, and moved copy controls into dedicated right-aligned rows for clearer, more responsive API key displays.
  • Localization

    • Replaced hardcoded texts with localized labels for save, copy, usage headings, and updated the revoke button wording.

@brin-security-scanner brin-security-scanner Bot added the contributor:flagged Contributor flagged for review by trust analysis. label Apr 27, 2026
@brin-security-scanner
Copy link
Copy Markdown

brin-security-scanner Bot commented Apr 27, 2026

⚠️ Contributor Trust Check — Review Recommended

This contributor's profile shows patterns that may warrant additional review. This is based on their GitHub activity, not the contents of this PR.

francmart514 · Score: 77/100

Dimension breakdown
Dimension Score What it measures
Identity 45 Account age, contribution history, GPG keys, org memberships
Behavior 90 PR patterns, unsolicited contribution ratio, activity cadence
Content 100 PR body substance, issue linkage, contribution quality
Graph 30 Cross-repo trust, co-contributor relationships

Analyzed by Brin · Full profile

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 27, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Apply layout fixes to settings and API key views to prevent horizontal overflow, make code snippets overflow-safe, move copy buttons to dedicated rows with i18n, and improve the clipboard controller to handle/reset timeouts and clear them on disconnect.

Changes

Cohort / File(s) Summary
Settings Layout
app/views/layouts/settings.html.erb
Add min-w-0 to main/inner wrappers; enable flex-wrap and reduce gap on sticky header row; replace shrink-0 with max-w-full for page-actions to avoid forced overflow.
API Key Templates
app/views/settings/api_keys/created.html.erb, app/views/settings/api_keys/created.turbo_stream.erb, app/views/settings/api_keys/show.html.erb, app/views/settings/api_keys/new.html.erb
Make API key and curl <code> blocks block-level and overflow-safe (min-w-0, overflow-x-auto, whitespace-pre-wrap); move copy buttons into right-aligned rows; add data-controller="clipboard" to curl copy elements; replace several hardcoded labels with i18n keys.
Clipboard Controller
app/javascript/controllers/clipboard_controller.js
showSuccess() now returns early if icon targets missing; introduce resetTimeout to clear prior timeouts before scheduling a 3s reset; add disconnect() to clear pending timeout on detach.
Locales
config/locales/views/settings/api_keys/en.yml
Add en.settings.api_keys.new.save_key and change en.settings.api_keys.show.revoke_key value to “Revoke API Key”.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

  • #284: Modifies layout flex/shrink classes in app/views/layouts/settings.html.erb (closely related to overflow/flex changes).
  • #653: Edits the settings layout template and responsive spacing/overflow classes.
  • #116: Touches API key views and clipboard UI, overlapping template and controller behavior.

Suggested labels

enhancement

Suggested reviewers

  • jjmata
  • matthieuEv

Poem

🐇 I hop through markup, tidy each row,
code blocks now scroll where long secrets go.
Buttons snug right, timeouts reset clean,
layout breathes easy — sleek and serene. 🥕

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix responsive issue in settings/api_key route page' directly and clearly describes the main change: fixing a responsive layout issue on the API Key settings page.
Linked Issues check ✅ Passed The PR addresses all coding requirements from issue #1565: fixed responsive layout in medium viewport widths (768px-987px), allowed proper content shrinking in split-pane layouts, refactored API key/copy button layout for stability, hardened containers against overflow, and updated test expectations.
Out of Scope Changes check ✅ Passed All changes are directly scoped to fixing the responsive layout issue: layout markup updates, API key UI restructuring, clipboard controller improvements, and localization strings are all related to the bug fix objectives.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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 and usage tips.

@brin-security-scanner brin-security-scanner Bot added the pr:verified PR passed security analysis. label Apr 27, 2026
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (3)
app/views/settings/api_keys/show.html.erb (2)

27-27: Confirm whitespace-pre-wrap + break-all is intentional on the key display.

break-all already wraps the long hex key inside the container, so overflow-x-auto on the same element will never engage — and whitespace-pre-wrap only matters if the rendered key contains whitespace (it shouldn't). Net effect is fine for a continuous hex string, but you can simplify to just break-all (or pick overflow-x-auto on a non-wrapping single-line variant) without changing visible behavior. Same on lines 118 and in created.html.erb / created.turbo_stream.erb.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/views/settings/api_keys/show.html.erb` at line 27, The API key display
uses redundant classes: remove "overflow-x-auto" and "whitespace-pre-wrap" and
keep only the wrapping behavior you want (e.g., "break-all") on the element with
id "api-key-display" in show.html.erb; apply the same simplification to the
equivalent element at line ~118 and the copies in created.html.erb and
created.turbo_stream.erb so the class list is minimal and consistent for
continuous hex keys.

25-46: Recommended: extract a shared partial for the API key display + curl snippet.

The same "API Key display + Copy button" block (lines 25–38) and "How to use your API key" curl block (lines 41–47) appear nearly verbatim three times across:

  • app/views/settings/api_keys/show.html.erb (newly-created branch, lines 25–47)
  • app/views/settings/api_keys/show.html.erb (existing-key branch, lines 116–138)
  • app/views/settings/api_keys/created.html.erb and created.turbo_stream.erb

Since this PR touches all four locations with identical Tailwind classes, a partial like _api_key_display.html.erb (taking api_key: as a local) would prevent the trio from drifting apart on the next styling tweak. Worth doing as a follow-up.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/views/settings/api_keys/show.html.erb` around lines 25 - 46, Duplicate
API key display and curl snippet repeated across templates; extract them into a
shared partial and render it with locals. Create a partial named
_api_key_display.html.erb that accepts locals api_key (string) and product_name
(or request_base_url if needed) and contains the code block with the <code>
element, the DS::Button render wired to clipboard#copy, and the curl snippet
using the passed-in base URL; then replace the duplicated blocks in
show.html.erb, the existing-key variant, created.html.erb and
created.turbo_stream.erb with render partial: "api_key_display", locals: {
api_key: `@current_api_key.plain_key`, product_name: product_name,
request_base_url: request.base_url } (or equivalent local names) so styling and
behavior remain identical and are maintained in one place.
app/views/settings/api_keys/created.html.erb (1)

84-86: Minor: redundant overflow-x-auto with break-all on curl container.

Since break-all already forces breaking within the long URL/key content, the container will never overflow horizontally and the overflow-x-auto scrollbar will not engage. You can keep one strategy or the other (e.g., drop break-all and rely on overflow-x-auto for a scrollable single-line snippet, which is often more readable for curl commands). Same applies to the <code> styling on lines 25–26 (break-all + overflow-x-auto).

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/views/settings/api_keys/created.html.erb` around lines 84 - 86, The curl
snippet container that renders 'curl -H "X-Api-Key:' and the related <code>
block use both "overflow-x-auto" and "break-all", which is redundant; pick one
strategy and remove the other. Locate the div with classes including
"bg-container rounded-lg p-3 font-mono text-sm text-primary border
border-primary max-w-full overflow-x-auto break-all" and remove "break-all" if
you want a single-line scrollable curl snippet (or remove "overflow-x-auto" if
you prefer forced wrapping); apply the same change to the corresponding <code>
element that currently combines "break-all" and "overflow-x-auto".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@app/views/settings/api_keys/created.html.erb`:
- Around line 84-86: The curl snippet container that renders 'curl -H
"X-Api-Key:' and the related <code> block use both "overflow-x-auto" and
"break-all", which is redundant; pick one strategy and remove the other. Locate
the div with classes including "bg-container rounded-lg p-3 font-mono text-sm
text-primary border border-primary max-w-full overflow-x-auto break-all" and
remove "break-all" if you want a single-line scrollable curl snippet (or remove
"overflow-x-auto" if you prefer forced wrapping); apply the same change to the
corresponding <code> element that currently combines "break-all" and
"overflow-x-auto".

In `@app/views/settings/api_keys/show.html.erb`:
- Line 27: The API key display uses redundant classes: remove "overflow-x-auto"
and "whitespace-pre-wrap" and keep only the wrapping behavior you want (e.g.,
"break-all") on the element with id "api-key-display" in show.html.erb; apply
the same simplification to the equivalent element at line ~118 and the copies in
created.html.erb and created.turbo_stream.erb so the class list is minimal and
consistent for continuous hex keys.
- Around line 25-46: Duplicate API key display and curl snippet repeated across
templates; extract them into a shared partial and render it with locals. Create
a partial named _api_key_display.html.erb that accepts locals api_key (string)
and product_name (or request_base_url if needed) and contains the code block
with the <code> element, the DS::Button render wired to clipboard#copy, and the
curl snippet using the passed-in base URL; then replace the duplicated blocks in
show.html.erb, the existing-key variant, created.html.erb and
created.turbo_stream.erb with render partial: "api_key_display", locals: {
api_key: `@current_api_key.plain_key`, product_name: product_name,
request_base_url: request.base_url } (or equivalent local names) so styling and
behavior remain identical and are maintained in one place.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: eb258840-b0ce-48b9-a9a6-3823bb5c4e57

📥 Commits

Reviewing files that changed from the base of the PR and between 3a4371f and a47fefa.

📒 Files selected for processing (5)
  • app/views/layouts/settings.html.erb
  • app/views/settings/api_keys/created.html.erb
  • app/views/settings/api_keys/created.turbo_stream.erb
  • app/views/settings/api_keys/show.html.erb
  • test/system/settings/api_keys_test.rb

Copy link
Copy Markdown
Collaborator

@jjmata jjmata left a comment

Choose a reason for hiding this comment

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

Thanks for this fix, @galuis116 / @francmart514. Swapping in the <YOUR_API_KEY> placeholder makes it so people can't just copy/paste the curl command and use it? Can you please put it back and add a "copy" icon as in the API key field above? We could trim those words since the copy icon is already universally understood by most people.

@francmart514
Copy link
Copy Markdown
Author

@jjmata I got it. I will update and push commit.

@francmart514
Copy link
Copy Markdown
Author

image

I have updated according to your feedback.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (2)
app/views/settings/api_keys/show.html.erb (2)

47-53: Inconsistent curl copy button across the three templates.

This file uses DS::Button for the curl copy control, while created.html.erb (lines 87–90) and created.turbo_stream.erb (lines 92–95) use a raw <button> with iconDefault/iconSuccess spans. Pick one shape and apply it everywhere — DS::Button is the more idiomatic choice given app/components-based design system and removes the now-orphaned icon-target markup in the other two files.

Also applies to: 147-153

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/views/settings/api_keys/show.html.erb` around lines 47 - 53, Replace the
raw HTML buttons in the created templates that use iconDefault/iconSuccess span
markup with the design-system component DS::Button so the curl-copy control is
consistent with show.html.erb; update the created.html.erb and
created.turbo_stream.erb templates to instantiate DS::Button with the same props
used in show.html.erb (text: "Copy", variant: "ghost", icon: "copy", class:
"shrink-0", data: { action: "clipboard#copy" }) and remove the now-orphaned
icon-target span elements and any related JS targets expecting those spans.

21-56: Consider extracting the duplicated key/curl blocks into a partial.

The “Your API Key” block (lines 21–39 and 121–139) and the “How to use your API key” block (lines 41–56 and 141–156) are essentially identical between the @newly_created and existing-key branches. Extracting them into _api_key_display.html.erb and _api_key_usage.html.erb partials taking the api_key local would cut ~70 lines and prevent the two copies from drifting (which already happened with the iconDefault/Success markup in created.*).

Also applies to: 121-156

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/views/settings/api_keys/show.html.erb` around lines 21 - 56, Extract the
duplicated API key display and usage blocks into two partials (e.g.
_api_key_display.html.erb and _api_key_usage.html.erb) that accept a local
api_key; move the first block (the container with code id="api-key-display", the
<code> showing `@current_api_key.plain_key`, and the DS::Button with data: {
action: "clipboard#copy" }) into _api_key_display.html.erb and the curl snippet
(the code showing curl -H "X-Api-Key: <%= api_key.plain_key %>" <%=
request.base_url %>/api/v1/accounts and its copy button) into
_api_key_usage.html.erb; then replace the duplicated inline blocks with render
partial: render "api_key_display", api_key: `@current_api_key` and render
"api_key_usage", api_key: `@current_api_key` (preserving the DS::Button
invocation, data-controller="clipboard", classes, and request.base_url usage) so
both the newly_created and existing-key branches share the single partials.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@app/javascript/controllers/clipboard_controller.js`:
- Around line 4-15: The copy affordance lost its success feedback because the
controller removed the iconDefault/iconSuccess targets and left the copy()
.then() empty; restore the public contract by adding back iconDefault and
iconSuccess to static targets and implement a showSuccess() method (or
equivalent) that toggles/hides iconDefaultTarget and shows iconSuccessTarget
briefly (or triggers the existing visual confirmation), then call showSuccess()
from the navigator.clipboard.writeText(...).then(...) callback in copy();
reference the static targets array, the copy(event) method, and the new
showSuccess() method when making the changes.

In `@app/views/settings/api_keys/created.turbo_stream.erb`:
- Around line 92-95: The copied curl button uses span targets
data-clipboard-target="iconDefault" and "iconSuccess" but the clipboard
controller (clipboard_controller.js) no longer declares those targets or the
showSuccess() behavior, so the success icon never appears; fix by restoring the
showSuccess() method and adding iconDefault and iconSuccess to the controller's
static targets (and any related timeout/visibility logic) so the button's spans
can be toggled, or alternatively remove the icon spans here and replace the
button markup with the DS::Button pattern used in show.html.erb (lines 47–53) to
keep behavior consistent with the updated controller.

---

Nitpick comments:
In `@app/views/settings/api_keys/show.html.erb`:
- Around line 47-53: Replace the raw HTML buttons in the created templates that
use iconDefault/iconSuccess span markup with the design-system component
DS::Button so the curl-copy control is consistent with show.html.erb; update the
created.html.erb and created.turbo_stream.erb templates to instantiate
DS::Button with the same props used in show.html.erb (text: "Copy", variant:
"ghost", icon: "copy", class: "shrink-0", data: { action: "clipboard#copy" })
and remove the now-orphaned icon-target span elements and any related JS targets
expecting those spans.
- Around line 21-56: Extract the duplicated API key display and usage blocks
into two partials (e.g. _api_key_display.html.erb and _api_key_usage.html.erb)
that accept a local api_key; move the first block (the container with code
id="api-key-display", the <code> showing `@current_api_key.plain_key`, and the
DS::Button with data: { action: "clipboard#copy" }) into
_api_key_display.html.erb and the curl snippet (the code showing curl -H
"X-Api-Key: <%= api_key.plain_key %>" <%= request.base_url %>/api/v1/accounts
and its copy button) into _api_key_usage.html.erb; then replace the duplicated
inline blocks with render partial: render "api_key_display", api_key:
`@current_api_key` and render "api_key_usage", api_key: `@current_api_key`
(preserving the DS::Button invocation, data-controller="clipboard", classes, and
request.base_url usage) so both the newly_created and existing-key branches
share the single partials.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 18a7e1cb-3907-451d-8cbd-358a99e51172

📥 Commits

Reviewing files that changed from the base of the PR and between a47fefa and dd3dd4e.

📒 Files selected for processing (4)
  • app/javascript/controllers/clipboard_controller.js
  • app/views/settings/api_keys/created.html.erb
  • app/views/settings/api_keys/created.turbo_stream.erb
  • app/views/settings/api_keys/show.html.erb

Comment thread app/javascript/controllers/clipboard_controller.js Outdated
Comment thread app/views/settings/api_keys/created.turbo_stream.erb Outdated
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

♻️ Duplicate comments (1)
app/views/settings/api_keys/created.html.erb (1)

84-95: ⚠️ Potential issue | 🟠 Major

Same curl-placeholder/whitespace-pre-wrap mismatch as created.turbo_stream.erb.

Line 85 still inlines <%= @api_key.plain_key %> and the <code> still carries whitespace-pre-wrap, despite the PR summary stating the curl was switched to an X-Api-Key: <YOUR_API_KEY> placeholder and that whitespace-pre-wrap was removed (with the system test updated to match). Please align this view with the rest of the change set so the system test stays green and behavior is consistent across the three API key views. The same comment about the redundant overflow-x-auto + break-all + whitespace-pre-wrap combo on lines 26 and 85 applies here.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/views/settings/api_keys/created.html.erb` around lines 84 - 95, The code
block still inlines `@api_key.plain_key` and retains the whitespace-pre-wrap
class; update the <code> element (the one with data-clipboard-target and
data-controller="clipboard") to use the placeholder X-Api-Key: <YOUR_API_KEY>
instead of <%= `@api_key.plain_key` %> and remove the whitespace-pre-wrap utility
from its class list (keep overflow-x-auto and break-all as needed); leave the
DS::Button (data: { action: "clipboard#copy" }) intact so clipboard behavior
remains unchanged.
🧹 Nitpick comments (1)
app/views/settings/api_keys/created.turbo_stream.erb (1)

30-41: Redundant overflow strategy on the API key block.

block w-full min-w-0 overflow-x-auto break-all whitespace-pre-wrap mixes a horizontal-scroll strategy (overflow-x-auto) with two wrapping strategies (break-all, whitespace-pre-wrap). The wrapping classes win, so the inner overflow-x-auto is effectively dead, and the outer overflow-hidden wrapper would clip a scrollbar anyway. Consider keeping just break-all (or break-words) for the key, and dropping whitespace-pre-wrap and the overflow plumbing to simplify. Also note whitespace-pre-wrap here will preserve any leading/trailing whitespace inside the ERB output — relying on <%= @api_key.plain_key %> being whitespace-clean.

♻️ Suggested simplification
-          <div class="bg-container rounded-lg p-3 border border-primary" data-controller="clipboard">
-            <div class="w-full overflow-hidden">
-              <code id="api-key-display" class="block w-full min-w-0 overflow-x-auto font-mono text-sm text-primary break-all whitespace-pre-wrap" data-clipboard-target="source"><%= `@api_key.plain_key` %></code>
-              <div class="mt-2 flex justify-end">
+          <div class="bg-container rounded-lg p-3 border border-primary min-w-0" data-controller="clipboard">
+            <code id="api-key-display" class="block w-full min-w-0 font-mono text-sm text-primary break-all" data-clipboard-target="source"><%= `@api_key.plain_key` %></code>
+            <div class="mt-2 flex justify-end">
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/views/settings/api_keys/created.turbo_stream.erb` around lines 30 - 41,
The API key block mixes conflicting overflow/wrapping strategies — update the
element with id "api-key-display" and its wrapper: remove the outer wrapper's
"overflow-hidden" and drop "overflow-x-auto" and "whitespace-pre-wrap" from the
<code id="api-key-display"> class list, leaving a single wrapping strategy such
as "break-all" (or "break-words") plus the existing sizing classes ("block
w-full min-w-0 font-mono text-sm text-primary") so long keys wrap correctly
without relying on scrollbars or preserved whitespace.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@app/views/settings/api_keys/created.html.erb`:
- Around line 84-95: The code block still inlines `@api_key.plain_key` and retains
the whitespace-pre-wrap class; update the <code> element (the one with
data-clipboard-target and data-controller="clipboard") to use the placeholder
X-Api-Key: <YOUR_API_KEY> instead of <%= `@api_key.plain_key` %> and remove the
whitespace-pre-wrap utility from its class list (keep overflow-x-auto and
break-all as needed); leave the DS::Button (data: { action: "clipboard#copy" })
intact so clipboard behavior remains unchanged.

---

Nitpick comments:
In `@app/views/settings/api_keys/created.turbo_stream.erb`:
- Around line 30-41: The API key block mixes conflicting overflow/wrapping
strategies — update the element with id "api-key-display" and its wrapper:
remove the outer wrapper's "overflow-hidden" and drop "overflow-x-auto" and
"whitespace-pre-wrap" from the <code id="api-key-display"> class list, leaving a
single wrapping strategy such as "break-all" (or "break-words") plus the
existing sizing classes ("block w-full min-w-0 font-mono text-sm text-primary")
so long keys wrap correctly without relying on scrollbars or preserved
whitespace.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 439ebf80-3c73-463f-a9d7-ee5bc4485b56

📥 Commits

Reviewing files that changed from the base of the PR and between dd3dd4e and cc38286.

📒 Files selected for processing (4)
  • app/javascript/controllers/clipboard_controller.js
  • app/views/settings/api_keys/created.html.erb
  • app/views/settings/api_keys/created.turbo_stream.erb
  • app/views/settings/api_keys/show.html.erb
🚧 Files skipped from review as they are similar to previous changes (1)
  • app/javascript/controllers/clipboard_controller.js

@francmart514 francmart514 requested a review from jjmata April 27, 2026 13:52
@jjmata jjmata removed the contributor:flagged Contributor flagged for review by trust analysis. label Apr 27, 2026
@jjmata jjmata added this to the v0.7.0 milestone Apr 27, 2026
@francmart514
Copy link
Copy Markdown
Author

waiting to merge

@brin-security-scanner brin-security-scanner Bot added the contributor:flagged Contributor flagged for review by trust analysis. label Apr 28, 2026
@francmart514
Copy link
Copy Markdown
Author

francmart514 commented Apr 28, 2026

Hello @jjmata I am waiting for you to check and merge this pull request.

@sure-admin sure-admin modified the milestones: v0.7.0, v0.7.1 Apr 29, 2026
@jjmata jjmata removed the contributor:flagged Contributor flagged for review by trust analysis. label Apr 29, 2026
@jjmata
Copy link
Copy Markdown
Collaborator

jjmata commented Apr 29, 2026

Broke it with my commit, will revisit. Just trying to use less copy/tooltips.

@brin-security-scanner brin-security-scanner Bot added the contributor:flagged Contributor flagged for review by trust analysis. label Apr 29, 2026
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@app/views/settings/api_keys/show.html.erb`:
- Line 44: The ERB view is embedding the real API key via
`@current_api_key.plain_key` into the example curl snippet; replace that
interpolation with a non-sensitive placeholder like "<API_KEY_HERE>" (keep
request.base_url as-is) in the <code class="block w-full min-w-0 overflow-x-auto
break-all whitespace-pre-wrap"> element(s) so the snippet shows a safe
placeholder instead of the actual value; update both occurrences that reference
`@current_api_key.plain_key`.
- Line 47: Replace the hardcoded "Copy" label in the API keys view with an i18n
lookup: change the text: "Copy" usage in
app/views/settings/api_keys/show.html.erb to use t('settings.api_keys.copy') (or
a similarly scoped key), and add that key with the English string to
config/locales/en.yml (e.g. settings: { api_keys: { copy: "Copy" } }) so the
button label is localizable across locales.
- Line 129: The i18n calls use the wrong namespace: change every occurrence of
t("settings.api_keys.current_api_key.*") in
app/views/settings/api_keys/show.html.erb (the t(...) calls at the reported
locations) to include the missing "show" segment so they become
t("settings.api_keys.show.current_api_key.*"); update the title call
(t("settings.api_keys.current_api_key.copy_key")) and the other two t(...)
usages at the same file to t("settings.api_keys.show.current_api_key.copy_key")
and the corresponding ".copied" / ".expires_at" keys respectively so they match
the translation scope.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: bad61257-5ea9-4eb1-b97a-fd4069756d32

📥 Commits

Reviewing files that changed from the base of the PR and between def934a and 6a100ea.

📒 Files selected for processing (3)
  • app/views/settings/api_keys/new.html.erb
  • app/views/settings/api_keys/show.html.erb
  • config/locales/views/settings/api_keys/en.yml
✅ Files skipped from review due to trivial changes (1)
  • app/views/settings/api_keys/new.html.erb

<div class="bg-container rounded-lg p-3 font-mono text-sm text-primary border border-primary">
curl -H "X-Api-Key: <%= @current_api_key.plain_key %>" <%= request.base_url %>/api/v1/accounts
<div class="bg-container rounded-lg p-3 font-mono text-sm text-primary border border-primary max-w-full overflow-hidden" data-controller="clipboard">
<code class="block w-full min-w-0 overflow-x-auto break-all whitespace-pre-wrap" data-clipboard-target="source">curl -H "X-Api-Key: <%= @current_api_key.plain_key %>" <%= request.base_url %>/api/v1/accounts</code>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Use a placeholder in curl snippets instead of interpolating the real key.

Lines 44 and 144 still embed @current_api_key.plain_key directly in the example command, which is risky in shared screens/logged DOM snapshots and weakens the overflow-hardening goal.

Proposed fix
-          <code class="block w-full min-w-0 overflow-x-auto break-all whitespace-pre-wrap" data-clipboard-target="source">curl -H "X-Api-Key: <%= `@current_api_key.plain_key` %>" <%= request.base_url %>/api/v1/accounts</code>
+          <code class="block w-full min-w-0 overflow-x-auto break-all whitespace-pre-wrap" data-clipboard-target="source">curl -H "X-Api-Key: &lt;YOUR_API_KEY&gt;" <%= request.base_url %>/api/v1/accounts</code>
...
-          <code class="block w-full min-w-0 overflow-x-auto break-all whitespace-pre-wrap" data-clipboard-target="source">curl -H "X-Api-Key: <%= `@current_api_key.plain_key` %>" <%= request.base_url %>/api/v1/accounts</code>
+          <code class="block w-full min-w-0 overflow-x-auto break-all whitespace-pre-wrap" data-clipboard-target="source">curl -H "X-Api-Key: &lt;YOUR_API_KEY&gt;" <%= request.base_url %>/api/v1/accounts</code>

Also applies to: 144-144

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/views/settings/api_keys/show.html.erb` at line 44, The ERB view is
embedding the real API key via `@current_api_key.plain_key` into the example curl
snippet; replace that interpolation with a non-sensitive placeholder like
"<API_KEY_HERE>" (keep request.base_url as-is) in the <code class="block w-full
min-w-0 overflow-x-auto break-all whitespace-pre-wrap"> element(s) so the
snippet shows a safe placeholder instead of the actual value; update both
occurrences that reference `@current_api_key.plain_key`.

<code class="block w-full min-w-0 overflow-x-auto break-all whitespace-pre-wrap" data-clipboard-target="source">curl -H "X-Api-Key: <%= @current_api_key.plain_key %>" <%= request.base_url %>/api/v1/accounts</code>
<div class="mt-2 flex justify-end">
<%= render DS::Button.new(
text: "Copy",
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Localize the new “Copy” button label.

Line 47 hardcodes user-facing text, which breaks localization in non-English locales.

Proposed fix
-              text: "Copy",
+              text: t(".current_api_key.copy_key"),

As per coding guidelines, "All user-facing strings MUST use the t() i18n helper function and corresponding locale keys in config/locales/en.yml."

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
text: "Copy",
text: t(".current_api_key.copy_key"),
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/views/settings/api_keys/show.html.erb` at line 47, Replace the hardcoded
"Copy" label in the API keys view with an i18n lookup: change the text: "Copy"
usage in app/views/settings/api_keys/show.html.erb to use
t('settings.api_keys.copy') (or a similarly scoped key), and add that key with
the English string to config/locales/en.yml (e.g. settings: { api_keys: { copy:
"Copy" } }) so the button label is localizable across locales.

Comment thread app/views/settings/api_keys/show.html.erb Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contributor:flagged Contributor flagged for review by trust analysis. pr:verified PR passed security analysis.

Development

Successfully merging this pull request may close these issues.

Bug: Not responsive in settings/api_key route age in midle width range

3 participants