QT/sure
1
0
Fork 0
mirror of https://github.com/we-promise/sure.git synced 2026-05-24 13:04:56 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(review): href-branch type-button bug + focus-ring tokens + profile Save submit

Browse Source 
CodeRabbit P1+P2 review on #1840:

1. button.rb: `merged_opts.delete(:href)` always returned nil because
   Buttonish#initialize strips :href from opts into @href, so the
   `if href.blank?` guard was ALWAYS true. Every DS::Button rendered via
   button_to (the href branch) got `type="button"` on the inner button,
   breaking submission of those button_to-generated forms (e.g.
   imports/_ready.html.erb publish button, imports/_failure.html.erb
   try-again button). Drop the local `href = merged_opts.delete(:href)`
   so the guard now reads the @href reader, leaving the href branch's
   HTML default intact.

2. settings/profiles/show.html.erb: the Save button is rendered with
   `render DS::Button.new(...)` inside `styled_form_with` (not via
   form.submit), so the StyledFormBuilder#submit type-pin from
   624e9794 doesn't cover it. Pass `type: :submit` explicitly so the
   profile form submits again under the default-type-button policy.

3. base.css: replace raw `outline-gray-900` / `outline-white` with the
   established alpha-ring focus pattern
   (focus-visible:ring-alpha-black-300 + theme-dark:ring-alpha-white-300)
   already used by app/components/settings/provider_card.html.erb and
   sure-design-system/components.css. Keeps a11y focus ring while using
   DS tokens.
This commit is contained in:
Guillem Arias
2026-05-19 16:26:08 +02:00
parent 624e9794ac
commit db563f3d62
3 changed files with 5 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
app/assets/tailwind/sure-design-system/base.css
View File

@@ -1,9 +1,9 @@
@layer base {
button {
@apply cursor-pointer focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-gray-900;
@apply cursor-pointer focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-offset-2 focus-visible:ring-alpha-black-300;
@variant theme-dark {
@apply focus-visible:outline-white;
@apply focus-visible:ring-alpha-white-300;
}
}

4
app/components/DS/button.rb
View File

@@ -22,7 +22,6 @@ class DS::Button < DS::Buttonish
def merged_opts
merged_opts = opts.dup || {}
extra_classes = merged_opts.delete(:class)
href = merged_opts.delete(:href)
data = merged_opts.delete(:data) || {}
if confirm.present?
@@ -37,7 +36,8 @@ class DS::Button < DS::Buttonish
# spec — meaning a DS::Button rendered inside a form will steal Enter-key
# submission from the first text input. Default to `type="button"` so
# callers must opt into submit behavior explicitly. `button_to` (href
# branch) wraps the button in its own form, so submit there is correct.
# branch) wraps the button in its own form, so submit there is correct
# and we leave its default alone.
if href.blank?
merged_opts[:type] ||= "button"
end

2
app/views/settings/profiles/show.html.erb
View File

@@ -19,7 +19,7 @@
</div>
<div class="flex justify-end mt-4">
<%= render DS::Button.new(text: t(".save"), class: "md:w-auto w-full justify-center") %>
<%= render DS::Button.new(text: t(".save"), type: :submit, class: "md:w-auto w-full justify-center") %>
</div>
</div>
<% end %>