Collapse rarely-changed config sections on org & person edit forms

[maebeale]

🤖 suggested review level: 3 Read 📖 mostly view-markup + small decorator methods; form fields stay in the DOM, submission behavior unchanged

What is the goal of this PR and why is this important?

• The org and person edit forms forced a long scroll past sections that rarely change. Collapse each to a one-line summary so the form is scannable; expand only what you need.

How did you approach the change?

• Native <details>/<summary> (matching the address-fields pattern) with an inline chevron beside the label/value and a hover cue — no JS.
• Summaries show the current value: sectors/categories/age-ranges (with the primary in bold + ⭐ and a sector leader 👑 + "(sector leader)", via *Decorator#sectors_summary / age_ranges_summary), addresses, emails, phones, licenses (names), and profile prefs as Hide phone and website / All shown. Background (person) shows Bio present / Shout-out present (shout-out styled admin-only).
• Open by default on new records / when there are errors so required or invalid fields aren't hidden; the rest collapse by default. Affiliations, Comments, and Communications are collapsible but expanded by default (the shared communications box became a <details>).
• Layout: org keeps name, Windows audience (collapsible, 50/50 with Sectors), Sectors, and Workshop settings in the left column beside a smaller logo (new size: local on the shared image field); Associated records moved below the save button and fits on one row. Person: Emails ‖ Phones parallel (secondary email wraps; contact-method row flex-wraps), name fields 2-up on mobile, demographics under a collapsible Extras header.

Fixes folded in

• Named the Affiliations/Addresses <details> groups so the section's group no longer triggers the inner tooltip / nested-field group-hover/group-open; added pointer-events-none to the affiliation hover tooltips so they can't block the collapse chevron.

Notes

• View/decorator/request specs updated and green (collapsed fields asserted with visible: :all; communications specs scope details not section).

[Copilot]

Pull request overview

This PR shortens the Organization edit experience by collapsing two long, rarely-edited sections into native <details>/<summary> accordions, preserving visibility of the current selections and keeping required/background fields discoverable on new records and validation errors.

Changes:

• Wrapped each organization category group (“Workshop settings” grid) in a <details> with a one-line summary showing the currently selected category names.
• Wrapped the “Background Info” block in a <details> that defaults open for new organizations and when the form has errors.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.

Suppressed comments (1)

app/views/organizations/_associated_records.html.erb:5

• nit: The new comment says this wraps to a “2/3-col grid” on narrower screens, but the classes now set sm:grid-cols-3 (no 2-column breakpoint). Updating the comment avoids future confusion when adjusting responsive layout.

<%# One row on md+ (grid-flow-col + equal auto columns); wraps to a 2/3-col grid
    on narrower screens. %>

[Copilot]

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated no new comments.

Suppressed comments (2)

app/views/people/_form.html.erb:442

• 🤖 From Copilot: should-fix: licenses_open doesn’t account for nested professional_license validation errors, so a failed save could re-render with the Professional licenses section collapsed and the invalid fields hidden.

  <% licenses_open = !f.object.persisted? || f.object.errors.any? %>

app/views/people/_form.html.erb:30

• 🤖 From Copilot: should-fix: contacts_open only checks f.object.errors, so a validation error on a nested contact_method can leave the Phones section collapsed and hide the inline error messages on re-render.

This issue also appears on line 442 of the same file.

        <% contacts_open = !f.object.persisted? || f.object.errors.any? %>
        <% phone_values = f.object.contact_methods.reject(&:marked_for_destruction?).filter_map { |cm| cm.value.presence } %>

[Copilot]

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated no new comments.

Suppressed comments (2)

app/views/people/_form.html.erb:443

• 🤖 From Copilot: should-fix: This section opens based on f.object.errors, but uniqueness/format errors can be attached to nested professional_licenses records; if those don’t bubble to the parent, the section can stay collapsed and hide the validation error.

  <% licenses = f.object.professional_licenses.reject(&:marked_for_destruction?) %>
  <% licenses_open = !f.object.persisted? || f.object.errors.any? %>
  <details class="form-group space-y-4 group"<%= " open".html_safe if licenses_open %>>

app/views/people/_form.html.erb:30

• 🤖 From Copilot: should-fix: The Emails/Phones sections only auto-open on f.object.errors, but validation errors can live on nested contact_methods records; that can leave the Phones section collapsed on a failed submit and hide the inline error messages.

This issue also appears on line 441 of the same file.

        <%# Emails and Phones sit parallel (50/50). Each collapses to the value(s)
            on file; both open on a new person or when there are errors. %>
        <% primary_email = f.object.user&.email.presence || f.object.email.presence %>
        <% email_summary = [ primary_email, f.object.email_2.presence ].compact %>
        <% contacts_open = !f.object.persisted? || f.object.errors.any? %>
        <% phone_values = f.object.contact_methods.reject(&:marked_for_destruction?).filter_map { |cm| cm.value.presence } %>

[Copilot]

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated no new comments.

Suppressed comments (2)

app/decorators/person_decorator.rb:115

• blocker: This age-range summary doesn’t exclude nested items marked for destruction, so after removing an age range and triggering a validation re-render, the collapsed summary can still show age ranges the user just removed.

  def age_ranges_summary
    items = object.age_range_items_ordered
    return "None selected" if items.empty?

app/decorators/application_decorator.rb:31

• blocker: This summary includes nested sector tags that are marked for destruction, so after removing a sector and hitting a validation error (re-render), the collapsed summary can still show sectors the user just removed.

    items = object.sectorable_items_primary_first.reject { |item| item.sector&.name == Sector::OTHER_SECTOR_NAME }

[Copilot]

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated no new comments.

Suppressed comments (1)

app/helpers/application_helper.rb:325

• 🤖 From Copilot: should-fix: This helper introduces single-quoted Ruby strings (e.g. in the wrap interpolations), which violates the repo’s "always use double quotes" RuboCop convention; switch these to double quotes to keep style consistent.

      label_tag = content_tag(:span, label, class: "font-medium #{text} #{wrap ? 'line-clamp-2' : 'truncate'}")

[Copilot]

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated no new comments.

Suppressed comments (2)

app/helpers/application_helper.rb:325

• should-fix: This adds single-quoted strings inside a Ruby expression; use double quotes for consistency with the repo’s string-literal convention.

      label_tag = content_tag(:span, label, class: "font-medium #{text} #{wrap ? 'line-clamp-2' : 'truncate'}")

app/helpers/application_helper.rb:314

• should-fix: This introduces a single-quoted string literal inside an interpolated class string; this codebase’s style guide prefers double quotes, and this can also trip RuboCop string-literal rules.

This issue also appears on line 325 of the same file.

                    #{'h-full min-h-[3.5rem]' if wrap}

[Copilot]

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

app/helpers/application_helper.rb:325

• 🤖 From Copilot: should-fix: This line uses single-quoted string literals ('line-clamp-2'/'truncate'), which violates the repo’s double-quote string convention and may trip RuboCop.

      label_tag = content_tag(:span, label, class: "font-medium #{text} #{wrap ? 'line-clamp-2' : 'truncate'}")

[Copilot]

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated no new comments.

Suppressed comments (2)

app/helpers/application_helper.rb:315

• 🤖 From Copilot: nit: This introduces single-quoted string literals inside a double-quoted string interpolation, which violates this repo’s RuboCop rule to always use double quotes for strings.

            class: "group flex items-center gap-3 w-full px-3 py-2 rounded-lg
                    #{'h-full min-h-[3.5rem]' if wrap}
                    border #{border} #{bg} #{hover_bg}

app/helpers/application_helper.rb:326

• 🤖 From Copilot: nit: This uses single quotes for string literals ('line-clamp-2'/'truncate'), but the project convention is double quotes everywhere, so RuboCop is likely to flag this.

      label_tag = content_tag(:span, label, class: "font-medium #{text} #{wrap ? 'line-clamp-2' : 'truncate'}")

[Copilot]

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated no new comments.

Suppressed comments (3)

app/helpers/application_helper.rb:316

• 🤖 From Copilot: should-fix: This introduces single-quoted string literals inside a file that enforces double-quoted strings (rubocop-rails-omakase), which is likely to fail lint.

    link_to path,
            data: { turbo_prefetch: false }.merge(data),
            class: "group flex items-center gap-3 w-full px-3 py-2 rounded-lg
                    #{'h-full min-h-[3.5rem]' if wrap}
                    border #{border} #{bg} #{hover_bg}
                    transition-colors duration-200 shadow-sm" do

app/helpers/application_helper.rb:326

• 🤖 From Copilot: should-fix: This uses single-quoted string literals inside a double-quoted interpolation, which violates the project’s double-quote string convention and can be avoided cleanly by extracting the conditional class.

      end

      label_tag = content_tag(:span, label, class: "font-medium #{text} #{wrap ? 'line-clamp-2' : 'truncate'}")

app/views/shared/_form_image_field.html.erb:58

• 🤖 From Copilot: should-fix: This line now touches the rounded class interpolation; using single-quoted string literals here conflicts with the repo’s enforced double-quote convention and can be rewritten without nested quotes.

        <%= image_tag file,
                      id: image_dom_id,
                      alt: "#{label}",
                      data: { file_preview_target: "preview" },
                      class: "#{size} object-cover border border-gray-300 shadow-sm #{rounded ? 'rounded-full' : ''}" %>

[Copilot]

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated no new comments.

Suppressed comments (1)

app/helpers/application_helper.rb:325

• nit: This introduces single-quoted strings inside index_button, but this repo’s RuboCop style requires double quotes for strings; switching these to double quotes avoids a style violation.

      label_tag = content_tag(:span, label, class: "font-medium #{text} #{wrap ? 'line-clamp-2' : 'truncate'}")

[Copilot]

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated no new comments.

Suppressed comments (2)

app/helpers/application_helper.rb:315

• 🤖 From Copilot: should-fix: This introduces a single-quoted string literal inside interpolation; the repo’s RuboCop config typically enforces double quotes and this will likely fail linting. Use double quotes for the conditional class string.

            class: "group flex items-center gap-3 w-full px-3 py-2 rounded-lg
                    #{'h-full min-h-[3.5rem]' if wrap}
                    border #{border} #{bg} #{hover_bg}

app/helpers/application_helper.rb:325

• 🤖 From Copilot: should-fix: Single quotes in the ternary ('line-clamp-2' / 'truncate') violate the project’s double-quote string convention and may trigger RuboCop failures.

      label_tag = content_tag(:span, label, class: "font-medium #{text} #{wrap ? 'line-clamp-2' : 'truncate'}")

[Copilot]

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated no new comments.

Suppressed comments (3)

app/helpers/application_helper.rb:315

• 🤖 From Copilot: nit: This introduces single-quoted string literals inside interpolation (e.g., #{'...' if wrap}), which conflicts with the repo’s documented “always use double quotes” Ruby style and may fail linting.

            class: "group flex items-center gap-3 w-full px-3 py-2 rounded-lg
                    #{'h-full min-h-[3.5rem]' if wrap}
                    border #{border} #{bg} #{hover_bg}

app/views/organizations/_form.html.erb:71

• 🤖 From Copilot: should-fix: windows_type select is rendered with label: false, which likely removes the associated <label> and can regress screen-reader accessibility; consider keeping an sr-only label (or at least an aria-label) even though the visible text is in the <summary>.

            <%= f.association :windows_type,
                              label: false,
                              include_blank: true,
                              required: false,
                              input_html: {

app/helpers/application_helper.rb:325

• 🤖 From Copilot: nit: Same single-quote style issue in the ternary; using double quotes here keeps the helper consistent with the repo’s Ruby string-literal convention.

      label_tag = content_tag(:span, label, class: "font-medium #{text} #{wrap ? 'line-clamp-2' : 'truncate'}")