mirror of
https://github.com/we-promise/sure.git
synced 2026-05-28 15:04:57 +00:00
refactor(views): migrate 6 residual inline alerts to DS::Alert (#1933)
* refactor(views): migrate 6 residual inline alerts to DS::Alert PR #1731 extended DS::Alert and migrated 9 inline alert blocks. Six hand-rolled alert blocks slipped through that sweep and stayed on raw palette tokens with no `theme-dark:` variants: - `app/views/settings/llm_usages/show.html.erb` — "About Cost Estimates" blue info block. Most visible offender: `bg-blue-50 border border-blue-200` + `text-blue-900 / text-blue-700 / text-blue-600` rendered as a bright white-blue island in dark mode (the bug spotted on the LLM usage page). - `app/views/accounts/confirm_unlink.html.erb` — yellow warning with bullet list. - `app/views/oidc_accounts/new_user.html.erb` — blue info heading. - `app/views/oidc_accounts/link.html.erb` — two blocks (yellow verify warning + blue create info). Also flips the file's pre-existing `text-gray-600` hint paragraph to `text-secondary` (caught by the `DeprecatedClasses` erb_lint rule on save). - `app/views/rules/confirm.html.erb` — AI cost notice. - `app/views/rules/confirm_all.html.erb` — AI cost notice. All six migrate to `DS::Alert.new(title:, variant:)` (with a block content slot for the rich/conditional bodies). DS::Alert resolves `bg-info/10`, `border-info/20`, etc. from the `@theme` semantic tokens, so dark mode now renders a subtle blue/yellow tint over the page surface instead of a hardcoded light-mode pill. Out of scope (left as-is, not alert-shaped): - `app/views/assistant_messages/_tool_calls.html.erb` — a tool-call display panel (not an alert; needs its own token sweep). - `app/views/import/rows/_form.html.erb` — inline cell-error tooltip (`bg-red-50 border border-red-200`) — also not alert-shaped; a future PR can swap it to `bg-destructive/10 border-destructive-subtle` once #1932 lands. Surfaced while scanning DS drift for the LLM usage page bug. Tracking issue: #1715 (closed but conceptually relevant) / #1911 (active drift patrol). * fix(oidc): keep alert description in <p>, retarget tests for DS::Alert title CI on #1933 caught three test failures introduced by migrating the two OIDC link alerts and the verify-redirect copy from hand-rolled `<h3>` / `<p>` markup to `DS::Alert`: 1. `OidcAccountsControllerTest#test_should_show_create_account_option_for_new_user` 2. `OidcAccountsControllerTest#test_does_not_show_create_account_button_when_JIT_link-only_mode` 3. `SessionsControllerTest#test_redirects_to_account_linking_when_no_OIDC_identity_exists` DS::Alert renders its `title:` slot as a `<p>` (semantically the alert heading lives on the container's `aria-labelledby`, not on a heading tag) and renders block / message content directly inside a `<div>`, not a `<p>`. The pre-migration markup used `<h3>` for the heading and `<p class="...text-blue-700">` for the description, so the tests above asserted those specific tags. Two fixes: - `app/views/oidc_accounts/link.html.erb` — wrap the html_safe description bodies in explicit `<p>` tags inside the DS::Alert block. Restores the `<p>` element the session-redirect test asserts on, and keeps the description as a semantic paragraph rather than a bare text node inside the alert container. - `test/controllers/oidc_accounts_controller_test.rb` — flip the two `assert_select "h3", text: "Create New Account"` calls to match the DS::Alert title `<p>`. The test was asserting an implementation detail of the pre-migration markup; switching to the new tag keeps the assertion meaningful (the heading text still has to render) without re-introducing an `<h3>` outside of DS::Alert. * fix(test): match Create New Account title with regex (sr-only "Info:" prefix) DS::Alert prepends `<span class="sr-only">Info:</span>` inside the title `<p>`, so the full text content is "Info: Create New Account", not "Create New Account". `assert_select "p", text: "Create New Account"` requires an exact text match and rejected the prefixed string. Switch to a regex match — keeps the heading-text assertion meaningful without coupling to the screen-reader prefix.
This commit is contained in:
committed by
GitHub
parent
f0e270f578
commit
ea51612ac7
@@ -6,19 +6,15 @@
|
||||
<%= t("accounts.confirm_unlink.description_html", account_name: @account.name, provider_name: @account.provider_name) %>
|
||||
</p>
|
||||
|
||||
<div class="mb-4 p-3 bg-yellow-50 border border-yellow-200 rounded-lg">
|
||||
<div class="flex items-start gap-2">
|
||||
<%= icon "alert-triangle", class: "w-4 h-4 text-yellow-600 mt-0.5 flex-shrink-0" %>
|
||||
<div class="text-xs">
|
||||
<p class="font-medium text-yellow-900 mb-1"><%= t("accounts.confirm_unlink.warning_title") %></p>
|
||||
<ul class="text-yellow-700 list-disc list-inside space-y-1">
|
||||
<li><%= t("accounts.confirm_unlink.warning_no_sync") %></li>
|
||||
<li><%= t("accounts.confirm_unlink.warning_manual_updates") %></li>
|
||||
<li><%= t("accounts.confirm_unlink.warning_transactions_kept") %></li>
|
||||
<li><%= t("accounts.confirm_unlink.warning_can_delete") %></li>
|
||||
</ul>
|
||||
</div>
|
||||
</div>
|
||||
<div class="mb-4">
|
||||
<%= render DS::Alert.new(title: t("accounts.confirm_unlink.warning_title"), variant: :warning) do %>
|
||||
<ul class="list-disc list-inside">
|
||||
<li><%= t("accounts.confirm_unlink.warning_no_sync") %></li>
|
||||
<li><%= t("accounts.confirm_unlink.warning_manual_updates") %></li>
|
||||
<li><%= t("accounts.confirm_unlink.warning_transactions_kept") %></li>
|
||||
<li><%= t("accounts.confirm_unlink.warning_can_delete") %></li>
|
||||
</ul>
|
||||
<% end %>
|
||||
</div>
|
||||
|
||||
<%= render DS::Button.new(
|
||||
|
||||
@@ -3,12 +3,11 @@
|
||||
%>
|
||||
|
||||
<% if @user_exists %>
|
||||
<div class="mb-6 p-4 bg-yellow-50 border border-yellow-200 rounded-md">
|
||||
<h3 class="text-sm font-medium text-yellow-800 mb-2"><%= t("oidc_accounts.link.verify_heading") %></h3>
|
||||
<p class="text-sm text-yellow-700">
|
||||
<div class="mb-6">
|
||||
<%= render DS::Alert.new(title: t("oidc_accounts.link.verify_heading"), variant: :warning) do %>
|
||||
<% email_suffix = @pending_auth["email"].present? ? t("oidc_accounts.link.email_suffix_html", email: @pending_auth["email"]) : "" %>
|
||||
<%= t("oidc_accounts.link.verify_description_html", provider: @pending_auth["provider"], email_suffix: email_suffix).html_safe %>
|
||||
</p>
|
||||
<p><%= t("oidc_accounts.link.verify_description_html", provider: @pending_auth["provider"], email_suffix: email_suffix).html_safe %></p>
|
||||
<% end %>
|
||||
</div>
|
||||
|
||||
<%= styled_form_with url: create_link_oidc_account_path, class: "space-y-4", data: { turbo: false } do |form| %>
|
||||
@@ -26,18 +25,17 @@
|
||||
placeholder: t("oidc_accounts.link.password_placeholder"),
|
||||
autocomplete: "current-password" %>
|
||||
|
||||
<div class="text-sm text-gray-600 mt-2">
|
||||
<div class="text-sm text-secondary mt-2">
|
||||
<p><%= t("oidc_accounts.link.verify_hint") %></p>
|
||||
</div>
|
||||
|
||||
<%= form.submit t("oidc_accounts.link.submit_link") %>
|
||||
<% end %>
|
||||
<% else %>
|
||||
<div class="mb-6 p-4 bg-blue-50 border border-blue-200 rounded-md">
|
||||
<h3 class="text-sm font-medium text-blue-800 mb-2"><%= t("oidc_accounts.link.create_heading") %></h3>
|
||||
<p class="text-sm text-blue-700">
|
||||
<%= t("oidc_accounts.link.create_description_html", email: @pending_auth["email"], provider: @pending_auth["provider"]).html_safe %>
|
||||
</p>
|
||||
<div class="mb-6">
|
||||
<%= render DS::Alert.new(title: t("oidc_accounts.link.create_heading"), variant: :info) do %>
|
||||
<p><%= t("oidc_accounts.link.create_description_html", email: @pending_auth["email"], provider: @pending_auth["provider"]).html_safe %></p>
|
||||
<% end %>
|
||||
</div>
|
||||
|
||||
<div class="space-y-4">
|
||||
|
||||
@@ -2,11 +2,12 @@
|
||||
header_title t("oidc_accounts.new_user.title")
|
||||
%>
|
||||
|
||||
<div class="mb-6 p-4 bg-blue-50 border border-blue-200 rounded-md">
|
||||
<h3 class="text-sm font-medium text-blue-800 mb-2"><%= t("oidc_accounts.new_user.heading") %></h3>
|
||||
<p class="text-sm text-blue-700">
|
||||
<%= t("oidc_accounts.new_user.description", provider: @pending_auth["provider"]) %>
|
||||
</p>
|
||||
<div class="mb-6">
|
||||
<%= render DS::Alert.new(
|
||||
title: t("oidc_accounts.new_user.heading"),
|
||||
message: t("oidc_accounts.new_user.description", provider: @pending_auth["provider"]),
|
||||
variant: :info
|
||||
) %>
|
||||
</div>
|
||||
|
||||
<%= styled_form_with model: @user, url: create_user_oidc_account_path, class: "space-y-4", data: { turbo: false } do |form| %>
|
||||
|
||||
@@ -15,31 +15,25 @@
|
||||
|
||||
<% if @rule.actions.any? { |a| a.action_type == "auto_categorize" } %>
|
||||
<% affected_count = @rule.affected_resource_count %>
|
||||
<div class="mb-4 p-3 bg-blue-50 border border-blue-200 rounded-lg">
|
||||
<div class="flex items-start gap-2">
|
||||
<%= icon "info", class: "w-4 h-4 text-blue-600 mt-0.5 flex-shrink-0" %>
|
||||
<div class="text-xs">
|
||||
<p class="font-medium text-blue-900 mb-1"><%= t(".ai_cost_title") %></p>
|
||||
<% if @estimated_cost.present? %>
|
||||
<p class="text-blue-700">
|
||||
<%= t(".ai_cost_with_estimate_html", count: affected_count, cost: sprintf("%.4f", @estimated_cost)) %>
|
||||
</p>
|
||||
<% else %>
|
||||
<p class="text-blue-700">
|
||||
<%= t(".ai_cost_no_estimate_html", count: affected_count) %>
|
||||
<% if @selected_model.present? %>
|
||||
<span class="font-semibold"><%= t(".cost_unavailable_model", model: @selected_model) %></span>
|
||||
<% else %>
|
||||
<span class="font-semibold"><%= t(".cost_unavailable_no_provider") %></span>
|
||||
<% end %>
|
||||
<%= t(".cost_warning") %>
|
||||
</p>
|
||||
<% end %>
|
||||
<p class="text-blue-600 mt-1">
|
||||
<%= link_to t(".view_usage_history"), settings_llm_usage_path, class: "underline hover:text-blue-800" %>
|
||||
<div class="mb-4">
|
||||
<%= render DS::Alert.new(title: t(".ai_cost_title"), variant: :info) do %>
|
||||
<% if @estimated_cost.present? %>
|
||||
<p><%= t(".ai_cost_with_estimate_html", count: affected_count, cost: sprintf("%.4f", @estimated_cost)) %></p>
|
||||
<% else %>
|
||||
<p>
|
||||
<%= t(".ai_cost_no_estimate_html", count: affected_count) %>
|
||||
<% if @selected_model.present? %>
|
||||
<span class="font-semibold"><%= t(".cost_unavailable_model", model: @selected_model) %></span>
|
||||
<% else %>
|
||||
<span class="font-semibold"><%= t(".cost_unavailable_no_provider") %></span>
|
||||
<% end %>
|
||||
<%= t(".cost_warning") %>
|
||||
</p>
|
||||
</div>
|
||||
</div>
|
||||
<% end %>
|
||||
<p>
|
||||
<%= link_to t(".view_usage_history"), settings_llm_usage_path, class: "text-link underline" %>
|
||||
</p>
|
||||
<% end %>
|
||||
</div>
|
||||
<% end %>
|
||||
|
||||
|
||||
@@ -9,32 +9,28 @@
|
||||
</p>
|
||||
|
||||
<% if @rules.any? { |r| r.actions.any? { |a| a.action_type == "auto_categorize" } } %>
|
||||
<div class="mb-4 p-3 bg-blue-50 border border-blue-200 rounded-lg">
|
||||
<div class="flex items-start gap-2">
|
||||
<%= icon "info", class: "w-4 h-4 text-blue-600 mt-0.5 flex-shrink-0" %>
|
||||
<div class="text-xs">
|
||||
<p class="font-medium text-blue-900 mb-1"><%= t("rules.apply_all.ai_cost_title") %></p>
|
||||
<% if @estimated_cost.present? %>
|
||||
<p class="text-blue-700">
|
||||
<%= t("rules.apply_all.ai_cost_message", transactions: @total_affected_count) %>
|
||||
<%= t("rules.apply_all.estimated_cost", cost: sprintf("%.4f", @estimated_cost)) %>
|
||||
</p>
|
||||
<% else %>
|
||||
<p class="text-blue-700">
|
||||
<%= t("rules.apply_all.ai_cost_message", transactions: @total_affected_count) %>
|
||||
<% if @selected_model.present? %>
|
||||
<span class="font-semibold"><%= t("rules.apply_all.cost_unavailable_model", model: @selected_model) %></span>
|
||||
<% else %>
|
||||
<span class="font-semibold"><%= t("rules.apply_all.cost_unavailable_no_provider") %></span>
|
||||
<% end %>
|
||||
<%= t("rules.apply_all.cost_warning") %>
|
||||
</p>
|
||||
<% end %>
|
||||
<p class="text-blue-600 mt-1">
|
||||
<%= link_to t("rules.apply_all.view_usage"), settings_llm_usage_path, class: "underline hover:text-blue-800" %>
|
||||
<div class="mb-4">
|
||||
<%= render DS::Alert.new(title: t("rules.apply_all.ai_cost_title"), variant: :info) do %>
|
||||
<% if @estimated_cost.present? %>
|
||||
<p>
|
||||
<%= t("rules.apply_all.ai_cost_message", transactions: @total_affected_count) %>
|
||||
<%= t("rules.apply_all.estimated_cost", cost: sprintf("%.4f", @estimated_cost)) %>
|
||||
</p>
|
||||
</div>
|
||||
</div>
|
||||
<% else %>
|
||||
<p>
|
||||
<%= t("rules.apply_all.ai_cost_message", transactions: @total_affected_count) %>
|
||||
<% if @selected_model.present? %>
|
||||
<span class="font-semibold"><%= t("rules.apply_all.cost_unavailable_model", model: @selected_model) %></span>
|
||||
<% else %>
|
||||
<span class="font-semibold"><%= t("rules.apply_all.cost_unavailable_no_provider") %></span>
|
||||
<% end %>
|
||||
<%= t("rules.apply_all.cost_warning") %>
|
||||
</p>
|
||||
<% end %>
|
||||
<p>
|
||||
<%= link_to t("rules.apply_all.view_usage"), settings_llm_usage_path, class: "text-link underline" %>
|
||||
</p>
|
||||
<% end %>
|
||||
</div>
|
||||
<% end %>
|
||||
|
||||
|
||||
@@ -165,16 +165,11 @@
|
||||
</div>
|
||||
</div>
|
||||
|
||||
<!-- Pricing Information -->
|
||||
<div class="mt-6 p-4 bg-blue-50 border border-blue-200 rounded-lg">
|
||||
<div class="flex items-start gap-2">
|
||||
<%= icon "info", class: "w-5 h-5 text-blue-600 mt-0.5" %>
|
||||
<div>
|
||||
<p class="text-sm font-medium text-blue-900"><%= t(".cost_estimates_title") %></p>
|
||||
<p class="text-xs text-blue-700 mt-1">
|
||||
<%= t(".cost_estimates_description") %>
|
||||
</p>
|
||||
</div>
|
||||
</div>
|
||||
<div class="mt-6">
|
||||
<%= render DS::Alert.new(
|
||||
title: t(".cost_estimates_title"),
|
||||
message: t(".cost_estimates_description"),
|
||||
variant: :info
|
||||
) %>
|
||||
</div>
|
||||
</div>
|
||||
|
||||
@@ -115,7 +115,7 @@ class OidcAccountsControllerTest < ActionController::TestCase
|
||||
|
||||
get :link
|
||||
assert_response :success
|
||||
assert_select "h3", text: "Create New Account"
|
||||
assert_select "p", text: /Create New Account/
|
||||
assert_select "strong", text: new_user_auth["email"]
|
||||
end
|
||||
|
||||
@@ -128,7 +128,7 @@ class OidcAccountsControllerTest < ActionController::TestCase
|
||||
get :link
|
||||
assert_response :success
|
||||
|
||||
assert_select "h3", text: "Create New Account"
|
||||
assert_select "p", text: /Create New Account/
|
||||
# No create account button rendered
|
||||
assert_select "button", text: "Create Account", count: 0
|
||||
assert_select "p", text: /New account creation via single sign-on is disabled/
|
||||
|
||||
Reference in New Issue
Block a user