admin: membership moves from the editor to the card
The editor edits the lines a group holds and nothing more:
its add button climbs to the caption above the card,
and the member checkboxes leave the form for good.
The card gets an "edit members" stub with no href until that screen exists.
Assisted-by: Claude Opus 5 via Claude Code
diff --git a/lib/pro_tacts/admin/groups_edit.rb b/lib/pro_tacts/admin/groups_edit.rb
index 7a3e3c4..002dd97 100644
--- a/lib/pro_tacts/admin/groups_edit.rb
+++ b/lib/pro_tacts/admin/groups_edit.rb
@@ -14,44 +14,38 @@ module ProTacts
# (Web#apply_group_edit). A group's version rides along hidden, the
# snapshot guard a contact's etag is on its editor.
#
- # Membership is checkboxes, native form state rather than script: a
- # member is a checked tag, unchecking one removes it on save, and
- # the add dialog lists every contact not yet in the group as an
- # unchecked box that submits with the form it sits outside of (the
- # `form` attribute) — the dialog being the view's one floated layer,
- # docs/DESIGN.md's rule for adding. A hidden empty `members[]`
- # makes the list present on every save of this form, so a POST that
- # carries no list at all is one that says nothing about membership
- # (#apply_edit's is-a-Hash posture) rather than one that empties it.
+ # Membership is edited from the group's card (Admin::GroupsShow),
+ # not here: it is a relationship, not a line the group holds. The
+ # form carries
+ # no `members[]`, so a save says nothing about membership
+ # (#apply_edit's is-a-Hash posture) and leaves it as it stands.
class GroupsEdit < Phlex::HTML
# @rbs @group: Store::Group
# @rbs @reading: Contact
- # @rbs @members: Array[Contact]
- # @rbs @others: Array[Contact]
# @rbs @notice: String?
- #: (group: Store::Group, contacts: Array[Contact], ?notice: String?) -> void
- def initialize(group:, contacts:, notice: nil)
+ #: (group: Store::Group, ?notice: String?) -> void
+ def initialize(group:, notice: nil)
@group = group
@reading = group.reading
- @members, @others = contacts.partition { group.members.include?(it.id) }
@notice = notice
end
def view_template
render Layout.new(title: "Edit #{@group.label}", notice: @notice) do
- div(class: "record") do
+ # The scope wraps the whole record because the add button sits
+ # in the caption row above the card, where the contact editor's
+ # add button sits, and the row it reveals is inside the form.
+ div(class: "record", x_data: "{ adding: false }") do
div(class: "record-nav") do
a(href: "/groups/#{@group.id}", class: "type-label") { "‹ #{@group.label}" }
- if @others.any?
- button(type: "button", data_size: "sm", popovertarget: "add-members") { "add members" }
- end
+ button(type: "button", data_size: "sm", "x-show": "!adding",
+ "@click": "adding = true") { "add address" }
end
div(class: "card") do
div(class: "card-body") do
- form(id: "group-form", action: "/groups/#{@group.id}", method: "post", class: "field-stack") do
+ form(action: "/groups/#{@group.id}", method: "post", class: "field-stack") do
input(type: "hidden", name: "version", value: @group.version)
- input(type: "hidden", name: "members[]", value: "")
# The id as the blank's placeholder, because a group
# with no name is shown as its id everywhere else.
label(class: "field") do
@@ -59,7 +53,6 @@ module ProTacts
input(type: "text", name: "name", value: @group.name,
placeholder: @group.id, autofocus: true)
end
- members_row if @members.any?
@reading.addresses.each { |address| address_row(address) }
added_address_row
# The first NOTE, the contact editor's own rule: this
@@ -78,27 +71,11 @@ module ProTacts
end
end
end
- add_members_dialog if @others.any?
end
end
private
- #: () -> void
- def members_row
- div(class: "field") do
- span { "Members" }
- div(class: "tag-set") do
- @members.each do |member|
- label(class: "tag") do
- input(type: "checkbox", name: "members[]", value: member.id, checked: true)
- plain(member.name || member.id)
- end
- end
- end
- end
- end
-
# ContactsEdit#address_row, over a group's line.
#: (Contact::Address address) -> void
def address_row(address)
@@ -113,47 +90,21 @@ module ProTacts
end
end
- # One address added per save, revealed on ask so no empty row
- # stands in the form until one is wanted (ContactsEdit#added_rows'
- # reason, and Alpine's). `display: contents` keeps both halves on
- # the form's own grid.
+ # One address added per save, revealed by the caption row's button
+ # so no empty row stands in the form until one is wanted
+ # (ContactsEdit#added_rows' reason, and Alpine's).
#: () -> void
def added_address_row
- div(x_data: "{ adding: false }", style: "display: contents;") do
- template("x-if": "adding") do
- div(class: "field") do
- span { "address" }
- div(class: "field-stack") do
- ContactsEdit::ADDRESS_FIELDS.each_with_index do |(component, label), index|
- input(type: "text", name: "new_address[0][#{component}]", placeholder: label,
- aria_label: label, "x-init": index.zero? ? "$el.focus()" : nil)
- end
+ template("x-if": "adding") do
+ div(class: "field") do
+ span { "address" }
+ div(class: "field-stack") do
+ ContactsEdit::ADDRESS_FIELDS.each_with_index do |(component, label), index|
+ input(type: "text", name: "new_address[0][#{component}]", placeholder: label,
+ aria_label: label, "x-init": index.zero? ? "$el.focus()" : nil)
end
end
end
- div(class: "field", "x-show": "!adding") do
- span
- button(type: "button", data_size: "sm", "@click": "adding = true") { "add address" }
- end
- end
- end
-
- #: () -> void
- def add_members_dialog
- dialog(id: "add-members", popover: "auto") do
- header { "Add members" }
- div(class: "field-stack") do
- @others.each do |contact|
- label do
- input(type: "checkbox", name: "members[]", value: contact.id, form: "group-form")
- plain(contact.name || contact.id)
- end
- end
- end
- footer do
- button(type: "button", popovertarget: "add-members",
- popovertargetaction: "hide") { "Done" }
- end
end
end
end
diff --git a/lib/pro_tacts/admin/groups_show.rb b/lib/pro_tacts/admin/groups_show.rb
index 9263137..26db87b 100644
--- a/lib/pro_tacts/admin/groups_show.rb
+++ b/lib/pro_tacts/admin/groups_show.rb
@@ -35,9 +35,9 @@ module ProTacts
div(class: "detail-header") do
h1(class: "type-h2", style: "margin: 0;") { @group.label }
end
- # ContactsShow#has_data?'s reason: an empty grid would
- # still take card-body's gap.
- dl(class: "detail-grid") { rows } if has_data?
+ # Never empty, unlike ContactsShow's: the members row
+ # always renders (#members_row).
+ dl(class: "detail-grid") { rows }
end
end
end
@@ -46,16 +46,11 @@ module ProTacts
private
- #: () -> bool
- def has_data?
- @members.any? || @reading.addresses.any? || @reading.notes.any?
- end
-
# Members first, the order a contact's card puts its groups in:
# the relationship frames the rows under it.
#: () -> void
def rows
- members_row if @members.any?
+ members_row
@reading.addresses.each do |address|
row(address.type || "address") do
Format.address_lines(address).each { |line| div { line } }
@@ -66,14 +61,22 @@ module ProTacts
end
end
+ # Rendered even with no members, because its link is the way to
+ # add one. Membership is edited here rather than in the editor,
+ # which edits only the lines the group holds. The link sits at the
+ # row's right edge where a lent row's badge sits (admin.css), and
+ # has no href until the members screen exists.
#: () -> void
def members_row
row("members") do
- div(class: "tag-set") do
- @members.each do |member|
- a(href: "/contacts/#{member.id}", class: "tag") { member.name || member.id }
+ if @members.any?
+ div(class: "tag-set") do
+ @members.each do |member|
+ a(href: "/contacts/#{member.id}", class: "tag") { member.name || member.id }
+ end
end
end
+ a(class: "type-label") { "edit members" }
end
end
diff --git a/lib/pro_tacts/web.rb b/lib/pro_tacts/web.rb
index 525bca3..d0f7773 100644
--- a/lib/pro_tacts/web.rb
+++ b/lib/pro_tacts/web.rb
@@ -869,7 +869,7 @@ module ProTacts
#: (Store::Group group, ?notice: String) -> String
def group_edit_screen(group, notice: nil)
response["Content-Type"] = "text/html; charset=utf-8"
- Admin::GroupsEdit.call(group:, contacts: store.contacts, notice:)
+ Admin::GroupsEdit.call(group:, notice:)
end
# A group's members as contacts, in the listing's own order.
diff --git a/public/admin.css b/public/admin.css
index e118714..43ed65c 100644
--- a/public/admin.css
+++ b/public/admin.css
@@ -552,7 +552,8 @@ img.avatar {
takes column 2 of the first row alone. No wrapper element to make
them one flex item, and no source-order inversion — the value is
still read before the group that lends it. */
-.detail-grid dd:has(> .badge) {
+.detail-grid dd:has(> .badge),
+.detail-grid dd:has(> a.type-label) {
display: grid;
grid-template-columns: minmax(0, 1fr) auto;
align-items: start;
@@ -574,9 +575,20 @@ img.avatar {
grid-row: 1;
min-height: calc(var(--gl-step--1) * 1.5);
}
-.detail-grid dd:has(> .badge) > div {
+.detail-grid dd:has(> .badge) > div,
+.detail-grid dd:has(> a.type-label) > div {
grid-column: 1;
}
+/* A row's action — a group card's "edit members" — takes the badge's
+ place at the row's right edge. Unlike the badge it is plain text,
+ so it has a real baseline to share with the tags beside it. */
+.detail-grid dd:has(> a.type-label) {
+ align-items: baseline;
+}
+.detail-grid dd > a.type-label {
+ grid-column: 2;
+ grid-row: 1;
+}
/* The stored card in its own collapsed card under the record — the
column's gap spaces the two, so no margin here. <details> is native,
diff --git a/sig/pro_tacts/admin.rbs b/sig/pro_tacts/admin.rbs
index 7eb6ff9..b3c6836 100644
--- a/sig/pro_tacts/admin.rbs
+++ b/sig/pro_tacts/admin.rbs
@@ -27,7 +27,7 @@ module ProTacts
end
class GroupsEdit
- def self.call: (group: Store::Group, contacts: Array[Contact], ?notice: String?) -> String
+ def self.call: (group: Store::Group, ?notice: String?) -> String
end
class DeviceSetup
diff --git a/test/pro_tacts/admin/test_groups_pages.rb b/test/pro_tacts/admin/test_groups_pages.rb
index 10575a4..ce736c3 100644
--- a/test/pro_tacts/admin/test_groups_pages.rb
+++ b/test/pro_tacts/admin/test_groups_pages.rb
@@ -105,10 +105,21 @@ class AdminGroupsPagesTest < Minitest::Test
assert_includes body, "Gate code 1854."
assert_includes body, %(<a href="/contacts/george" class="tag">George Boole</a>)
assert_includes body, %(<a href="/contacts/mary" class="tag">Mary Boole</a>)
+ assert_includes body, "edit members"
refute_includes body, "Ada Lovelace"
end
end
+ def test_a_memberless_card_still_offers_to_edit_members
+ with_contacts(BOOLES) do |store|
+ id = household(store, members: [])
+
+ get "/groups/#{id}"
+
+ assert_includes last_response.body, "edit members"
+ end
+ end
+
def test_an_unknown_group_is_a_404
with_contacts({}) do
get "/groups/zzzz"
@@ -121,7 +132,7 @@ class AdminGroupsPagesTest < Minitest::Test
## The editor
- def test_the_editor_checks_the_members_and_offers_everyone_else
+ def test_the_editor_leaves_membership_to_the_card
with_contacts(BOOLES) do |store|
id = household(store)
@@ -129,8 +140,9 @@ class AdminGroupsPagesTest < Minitest::Test
body = last_response.body
assert_includes body, %(<input type="hidden" name="version" value="#{store.group(id).version}">)
- assert_includes body, %(<input type="checkbox" name="members[]" value="george" checked>)
- assert_includes body, %(<input type="checkbox" name="members[]" value="ada" form="group-form">)
+ refute_includes body, "George Boole"
+ refute_includes body, "edit members"
+ refute_includes body, %(name="members[]")
assert_includes body, %(name="address[)
end
end