Skip to content

Commit 0776d52

Browse files
authored
Guard the alert kind, and let long alert text run the full width (#4023)
Follow-up to #4020. An unknown alert kind used to fall back to notice silently, so a caller that passed a typo'd or unhandled kind got a blue box and no signal. And a dismissable alert's text could only wrap inside its own column, so a long message pushed the dismiss button onto a line of its own. - **`kind` defaults to `:notice`, and anything outside `KINDS` raises.** An omitted kind and a wrong one used to be the same input; now only the wrong one is an error. Deployed environments notify Honeybadger and fall back instead of raising — the flash is swept after the layout renders, so a raise from there would raise again on every subsequent request. That's `production? || sandbox?`, per the gotcha in `config/environments/sandbox.rb`. - **`UI::Alerts::FlashMessage` drops its own copy of that guard** and hands the flash type straight to the alert. - **The icon and dismiss button float rather than sitting in a flex row**, so an alert's first line renders between them and every line after runs the full width. A float displaces only the lines it overlaps, which a flex item can't do — its text is stuck in its own column. Both are kept to about a line tall and precede the text, which is what floats require; the dismiss button moving above the message in the DOM also puts it first in tab order. - **Preview reorganization**: the warning kind variant is now the header variant, the separate header group is gone, and the dismissable group picks up a header variant and a long-text purple one that exercises the wrapping above. The `MARKUP_DIGEST` bumps are these edits reaching templates inside fragment caches.
1 parent 4568ea2 commit 0776d52

9 files changed

Lines changed: 79 additions & 53 deletions

File tree

app/components/admin/bikes_table/component.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ module BikesTable
88
# checkboxes, and skip_user to drop the owner column.
99
class Component < ApplicationComponent
1010
# Digest of the markup inside the row cache — the cached_markup_digest spec keeps it current
11-
MARKUP_DIGEST = "864a82f880b3"
11+
MARKUP_DIGEST = "96fa3d808254"
1212

1313
def initialize(bikes:, no_show_header: false, show_serial: false, render_sortable: false,
1414
skip_user: false, render_multi_check: false)

app/components/admin/users_table/component.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ module UsersTable
66
# render_deleted to show the deleted_at column.
77
class Component < ApplicationComponent
88
# Digest of the markup inside the row cache — the cached_markup_digest spec keeps it current
9-
MARKUP_DIGEST = "bc1f91c7d43d"
9+
MARKUP_DIGEST = "ffd7f9f91f03"
1010

1111
def initialize(users:, render_sortable: false, render_deleted: false)
1212
@users = users

app/components/registrations/show/wrapper/component.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ module Wrapper
88
class Component < ApplicationComponent
99
# Digest of the markup inside the cache block — the cached_markup_digest spec
1010
# keeps it current, following what this tree renders out into UI:: and elsewhere
11-
MARKUP_DIGEST = "2c440bc315b8"
11+
MARKUP_DIGEST = "86faa178a90f"
1212

1313
def initialize(bike:, current_user:, view:, available_views:, bike_sticker: nil, current_alerts: {})
1414
@bike = bike
Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,33 +1,22 @@
11
<div
2-
class="tw:flex tw:flex-wrap tw:items-baseline tw:gap-x-4 tw:gap-y-2 tw:rounded-sm tw:border tw:p-4 tw:transition-all tw:duration-300 <%= @margin_classes %> <%= color_classes %>"
2+
class="tw:flow-root tw:rounded-sm tw:border tw:p-4 tw:transition-all tw:duration-300 <%= @margin_classes %> <%= color_classes %>"
33
role="alert"
44
data-controller="ui--alert"
55
>
6-
<div class="tw:w-4">
6+
<div class="tw:float-left tw:mr-4 tw:w-4 <%= icon_classes %>">
77
<% if @icon.present? %>
88
<%= @icon %>
99
<% else %>
10-
<%= helpers.inline_svg_tag("icons/info.svg", class: "tw:-mb-0.5 tw:h-4 tw:w-4 tw:shrink-0", aria_hidden: true) %>
10+
<%= helpers.inline_svg_tag("icons/info.svg", class: "tw:-mb-0.5 tw:h-4 tw:w-4", aria_hidden: true) %>
1111
<% end %>
1212

1313
<span class="tw:sr-only"><%= @kind.to_s.titleize %></span>
1414
</div>
1515

16-
<% if @header.present? %>
17-
<h4 class="uncap tw:mb-0! tw:flex-1 tw:text-lg! <%= text_color_classes_important %>">
18-
<%= @header %>
19-
</h4>
20-
<% end %>
21-
22-
<div class="<%= body_classes %>">
23-
<%= @text %>
24-
<%= content %>
25-
</div>
26-
2716
<% if @dismissable %>
2817
<button
2918
type="button"
30-
class="tw:-mx-1.5 tw:-my-1.5 tw:inline-flex tw:h-8 tw:w-8 tw:items-center tw:justify-center tw:rounded-sm tw:p-1.5 tw:focus:ring-2 <%= dismissable_color_classes %>"
19+
class="tw:float-right tw:-my-1.5 tw:-mr-1.5 tw:ml-4 tw:inline-flex tw:h-8 tw:w-8 tw:items-center tw:justify-center tw:rounded-sm tw:p-1.5 tw:focus:ring-2 <%= dismissable_color_classes %>"
3120
aria-label="Close"
3221
data-action="click->ui--alert#close"
3322
>
@@ -50,4 +39,15 @@
5039
</svg>
5140
</button>
5241
<% end %>
42+
43+
<% if @header.present? %>
44+
<h4 class="uncap tw:mb-2! tw:text-lg! <%= text_color_classes_important %>">
45+
<%= @header %>
46+
</h4>
47+
<% end %>
48+
49+
<div>
50+
<%= @text %>
51+
<%= content %>
52+
</div>
5353
</div>

app/components/ui/alerts/base/component.rb

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -15,24 +15,32 @@ class Component < ApplicationComponent
1515

1616
# icon: rendered markup, e.g. inline_svg_tag("icons/envelope.svg", class: "tw:h-4 tw:w-4") -
1717
# replaces the default info icon
18-
def initialize(text: nil, header: nil, kind: nil, dismissable: false, margin_classes: "tw:mb-4", icon: nil)
18+
def initialize(text: nil, header: nil, kind: :notice, dismissable: false, margin_classes: "tw:mb-4", icon: nil)
1919
@text = text
2020
@header = header
21-
@kind = if KINDS.include?(kind&.to_sym)
22-
kind&.to_sym
23-
else
24-
KINDS.first
25-
end
21+
@kind = normalized_kind(kind)
2622
@dismissable = dismissable
2723
@margin_classes = margin_classes
2824
@icon = icon
2925
end
3026

3127
private
3228

33-
# order-last keeps the dismiss button, which is last in the DOM, up on the header row
34-
def body_classes
35-
@header.present? ? "tw:order-last tw:basis-full" : "tw:grow"
29+
def normalized_kind(kind)
30+
return kind.to_sym if KINDS.include?(kind&.to_sym)
31+
32+
unless Rails.env.production?
33+
raise ArgumentError, "unknown kind #{kind.inspect}, expected one of: #{KINDS.join(", ")}"
34+
end
35+
36+
Honeybadger.notify("Unknown alert kind", {error_class: self.class.to_s, context: {kind:}})
37+
KINDS.first
38+
end
39+
40+
# A float's box sits at the top of the line it shares, not on that line's
41+
# baseline, so nudge it down onto one -- further for a header
42+
def icon_classes
43+
@header.present? ? "tw:mt-[7px]" : "tw:mt-1"
3644
end
3745

3846
def color_classes

app/components/ui/alerts/base/component_preview.rb

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ module Alerts
55
module Base
66
class ComponentPreview < ApplicationComponentPreview
77
CONFIRMATION_TEXT = "We've sent a confirmation link to your email. No need to wait — you can finish registering right now."
8+
LONG_TEXT = "Bauhaus williamsburg seitan rams 8-bit live-edge edison bulb pork belly chia. Black trumpet locavore DSA wabi-sabi fitzcarraldo angela davis. Cassavetes dad shoes japanese breakfast gatekeep truffaut, offal wolf."
89

910
# @!group Kind variants
1011
def notice
@@ -15,8 +16,8 @@ def error
1516
render(UI::Alerts::Base::Component.new(text: "This is an error alert", kind: :error))
1617
end
1718

18-
def warning
19-
render(UI::Alerts::Base::Component.new(text: "This is a warning alert", kind: :warning))
19+
def with_header
20+
render(UI::Alerts::Base::Component.new(header: "Registration incomplete", kind: :warning, text: CONFIRMATION_TEXT))
2021
end
2122

2223
def success
@@ -34,17 +35,6 @@ def custom_icon
3435
end
3536
# @!endgroup
3637

37-
# @!group Header variants
38-
def with_header
39-
render(UI::Alerts::Base::Component.new(header: "Registration incomplete", kind: :warning, text: CONFIRMATION_TEXT))
40-
end
41-
42-
def dismissable_with_header
43-
render(UI::Alerts::Base::Component.new(header: "Registration incomplete", kind: :warning, text: CONFIRMATION_TEXT,
44-
dismissable: true))
45-
end
46-
# @!endgroup
47-
4838
# @!group Dismissable variants
4939
def dismissable_notice
5040
render(UI::Alerts::Base::Component.new(text: "Dismissable notice", kind: :notice, dismissable: true))
@@ -53,6 +43,15 @@ def dismissable_notice
5343
def dismissable_error
5444
render(UI::Alerts::Base::Component.new(text: "Dismissable error", kind: :error, dismissable: true))
5545
end
46+
47+
def dismissable_purple
48+
render(UI::Alerts::Base::Component.new(text: LONG_TEXT, kind: :purple, dismissable: true))
49+
end
50+
51+
def dismissable_with_header
52+
render(UI::Alerts::Base::Component.new(header: "Registration incomplete", kind: :warning, text: CONFIRMATION_TEXT,
53+
dismissable: true))
54+
end
5655
# @!endgroup
5756
end
5857
end

app/components/ui/alerts/flash_message/component.rb

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -15,18 +15,9 @@ def initialize(flash: {})
1515
def messages
1616
@flash.filter_map do |type, message|
1717
next unless message.is_a?(String)
18-
{text: message, kind: kind_for(type)}
18+
{text: message, kind: type}
1919
end
2020
end
21-
22-
# Rails sweeps the flash after the layout renders, so raising here would raise again
23-
# on the next request too -- keep it to the environments where that's a useful signal.
24-
def kind_for(type)
25-
kind = type.to_sym
26-
return kind if UI::Alerts::Base::Component::KINDS.include?(kind)
27-
raise ArgumentError, "Unknown flash type: #{type}" if Rails.env.local?
28-
UI::Alerts::Base::Component::KINDS.first
29-
end
3021
end
3122
end
3223
end

spec/components/ui/alerts/base/component_spec.rb

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
let(:options) { {text: "some text"} }
77
let(:component) { render_inline(described_class.new(**options)) }
88

9-
it "renders" do
9+
it "renders, defaulting to the notice kind" do
1010
expect(component).to be_present
1111
expect(component).to have_css('[role="alert"].tw:text-blue-800')
1212
expect(component.to_html).to include "M10 9.25v4.5" # the default info icon
@@ -55,6 +55,34 @@
5555
end
5656
end
5757

58+
describe "unknown kind" do
59+
let(:options) { {text: "some text", kind: "bogus"} }
60+
61+
it "raises" do
62+
expect { component }.to raise_error(ArgumentError, /unknown kind/i)
63+
end
64+
65+
context "passed nil, rather than omitted" do
66+
let(:options) { {text: "some text", kind: nil} }
67+
68+
it "raises" do
69+
expect { component }.to raise_error(ArgumentError, /unknown kind/i)
70+
end
71+
end
72+
73+
context "in production" do
74+
before { allow(Rails).to receive(:env).and_return("production".inquiry) }
75+
76+
it "renders a notice and notifies" do
77+
stub_const("Honeybadger", spy("Honeybadger"))
78+
79+
expect(component).to have_css('[role="alert"].tw:text-blue-800')
80+
expect(Honeybadger).to have_received(:notify)
81+
.with("Unknown alert kind", hash_including(context: {kind: "bogus"}))
82+
end
83+
end
84+
end
85+
5886
context "success dismissable" do
5987
let(:options) { {text: "some text", kind: "success", dismissable: true} }
6088
it "renders with dismissable" do

spec/components/ui/alerts/flash_message/component_spec.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,8 @@
4848
context "with an unknown flash type" do
4949
let(:flash) { {bogus: "Who knows"} }
5050

51-
it "raises" do
52-
expect { component }.to raise_error(ArgumentError, /unknown flash type/i)
51+
it "raises, from the kind it hands the alert" do
52+
expect { component }.to raise_error(ArgumentError, /unknown kind/i)
5353
end
5454
end
5555
end

0 commit comments

Comments
 (0)