From 057d71cf70f6079aeef2506b3014e549a3f05774 Mon Sep 17 00:00:00 2001 From: LaRita Robinson Date: Wed, 15 Jul 2026 11:40:47 -0400 Subject: [PATCH 1/7] WIP: typeahead + multi-select for controlled compounds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add two opt-in `form:` flags for a compound `controlled` member: `autocomplete: true` binds a select2 typeahead over the options (filtered client-side, since the full list is already in the DOM), and `multiple: true` renders a multi-select. Both default off, so existing controlled members are unchanged. A `multiple` member fans each selected value out into its own compound entry at submit time, sharing the row's other members. This keeps every stored member value a scalar — consumers never branch on array-vs-scalar, and each term indexes and facets on its own. The indexer and show renderer still tolerate a transient array value, since an invalid submission is echoed back before fan-out. WIP: specs are written but not yet run green in a stack; the Enact profile opt-in and the PR are follow-ups. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../javascripts/hyrax/compound_metadata.js | 29 ++++++- .../concerns/hyrax/compound_field_behavior.rb | 59 +++++++++++---- app/helpers/hyrax/compound_fields_helper.rb | 18 +++-- .../hyrax/indexers/compound_indexer.rb | 5 +- .../renderers/compound_attribute_renderer.rb | 10 +++ app/services/hyrax/compound_schema.rb | 8 +- .../hyrax/compound_subproperty_labeler.rb | 1 + .../hyrax/compounds/_compound_row.html.erb | 19 ++++- config/locales/hyrax.en.yml | 1 + documentation/compound_fields.md | 32 +++++++- .../hyrax/compound_field_behavior_spec.rb | 48 +++++++++++- .../hyrax/compound_fields_helper_spec.rb | 26 +++++++ .../hyrax/indexers/compound_indexer_spec.rb | 13 ++++ .../compound_attribute_renderer_spec.rb | 9 +++ spec/services/hyrax/compound_schema_spec.rb | 19 +++++ .../_compound_row_controlled_multiple_spec.rb | 75 +++++++++++++++++++ 16 files changed, 343 insertions(+), 29 deletions(-) create mode 100644 spec/views/hyrax/compounds/_compound_row_controlled_multiple_spec.rb diff --git a/app/assets/javascripts/hyrax/compound_metadata.js b/app/assets/javascripts/hyrax/compound_metadata.js index f1445cd09e..2b8ccf6734 100644 --- a/app/assets/javascripts/hyrax/compound_metadata.js +++ b/app/assets/javascripts/hyrax/compound_metadata.js @@ -67,6 +67,29 @@ }); } + // Bind select2 to a `controlled` sub-property tagged + // `data-hyrax-compound-controlled` (see _compound_row.html.erb). Unlike the + // work_or_url / linked_record pickers, this binds a NATIVE `: an inline `values:` - # list when present, otherwise the named QA authority. A stored value not - # among the options is appended so it still renders (`include_current_value`). + # list when present, otherwise the named QA authority. Any stored value not + # among the options is appended so it still renders. +current_value+ may be + # an array (a `multiple: true` member), in which case every stored value is + # ensured. # # @return [Array] `[[label, id], ...]` def compound_subproperty_options(spec, current_value = nil) options = spec[:values].presence || authority_options(spec[:authority]) - ensure_current_value(options, current_value) + Array(current_value).reject(&:blank?).reduce(options) { |opts, value| ensure_current_value(opts, value) } end ## - # @return [Boolean] whether +current_value+ is present but not among the + # @return [Boolean] whether any present +current_value+ is not among the # sub-property's offered options — i.e. a forced/stale value. The select # gets the +force-select+ class in that case, matching the ordinary - # controlled-field convention. + # controlled-field convention. Handles an array value (a `multiple: true` + # member): forced when any selected value is off-list. def compound_subproperty_forced?(spec, current_value = nil) - return false if current_value.blank? + values = Array(current_value).reject(&:blank?) + return false if values.empty? base = spec[:values].presence || authority_options(spec[:authority]) - base.none? { |(_label, id)| id.to_s == current_value.to_s } + values.any? { |value| base.none? { |(_label, id)| id.to_s == value.to_s } } end ## diff --git a/app/indexers/hyrax/indexers/compound_indexer.rb b/app/indexers/hyrax/indexers/compound_indexer.rb index d3c9d95d12..f2f0d623d5 100644 --- a/app/indexers/hyrax/indexers/compound_indexer.rb +++ b/app/indexers/hyrax/indexers/compound_indexer.rb @@ -64,7 +64,10 @@ def index_searchable_subproperties(document, definition, rows, compound_name) index_keys = solr_index_keys(compound_name, sub_property, spec) next if index_keys.empty? - values = rows.map { |row| compound_entry_value(row, sub_property) }.reject(&:blank?) + # `flatten` so a member whose stored value is itself an array (e.g. a + # `multiple: true` member echoed back before fan-out) contributes each + # term as its own facet value rather than a nested array. + values = rows.flat_map { |row| compound_entry_value(row, sub_property) }.reject(&:blank?) next if values.empty? index_keys.each { |index_key| document[index_key] = values } diff --git a/app/renderers/hyrax/renderers/compound_attribute_renderer.rb b/app/renderers/hyrax/renderers/compound_attribute_renderer.rb index 9c31673270..047ab8a407 100644 --- a/app/renderers/hyrax/renderers/compound_attribute_renderer.rb +++ b/app/renderers/hyrax/renderers/compound_attribute_renderer.rb @@ -106,7 +106,11 @@ def linked_record_markup(sub_property, value) # stored value is itself a linkable URI (e.g. a rights-statement or license # URI), link the term to that URI — mirroring the ordinary rights/license # renderer. Non-URI controlled values (e.g. inline option ids) stay plain. + # An array value (a `multiple: true` member echoed back before fan-out) + # renders each term, comma-separated. def controlled_markup(sub_property, value) + return safe_join_terms(Array(value).map { |v| controlled_markup(sub_property, v) }) if value.is_a?(::Array) + label = display_value(sub_property, value) if Hyrax::AuthorityRenderingHelper.linkable_uri?(value) %(#{ERB::Util.h(label)}).html_safe @@ -115,6 +119,12 @@ def controlled_markup(sub_property, value) end end + # Join already-escaped term fragments with a comma separator, preserving + # html_safe-ness. + def safe_join_terms(fragments) + fragments.map(&:to_s).join(', ').html_safe + end + # Link a URL or a resolvable work; render anything else as plain text so # we never emit a broken link to a non-existent work. def work_or_url_markup(value) diff --git a/app/services/hyrax/compound_schema.rb b/app/services/hyrax/compound_schema.rb index 66b80aba71..e777ea11a6 100644 --- a/app/services/hyrax/compound_schema.rb +++ b/app/services/hyrax/compound_schema.rb @@ -262,7 +262,13 @@ def normalize_subproperty(opts) required: truthy?(opts['required']), group: opts['group']&.to_s, cols: (form['cols'] || DEFAULT_COLS).to_i, - as: form['as']&.to_s }.merge(linked_record_keys(opts)) + as: form['as']&.to_s, + # `controlled`-only form opt-ins: `autocomplete` binds a select2 + # typeahead over the options; `multiple` allows several values, each + # fanned out into its own compound entry at submit time (see + # Hyrax::CompoundFieldBehavior). Both default false. + autocomplete: truthy?(form['autocomplete']), + multiple: truthy?(form['multiple']) }.merge(linked_record_keys(opts)) end # The `linked_record`-specific declarations: profile-driven lookup-or-create diff --git a/app/services/hyrax/compound_subproperty_labeler.rb b/app/services/hyrax/compound_subproperty_labeler.rb index caed10e306..074ecb187e 100644 --- a/app/services/hyrax/compound_subproperty_labeler.rb +++ b/app/services/hyrax/compound_subproperty_labeler.rb @@ -15,6 +15,7 @@ class CompoundSubpropertyLabeler # # @return [String] the term to display def self.label_for(spec, value) + return Array(value).map { |v| label_for(spec, v) }.join(', ') if value.is_a?(::Array) return value.to_s if spec.nil? || spec[:type].to_s != 'controlled' || value.blank? if spec[:values].present? diff --git a/app/views/hyrax/compounds/_compound_row.html.erb b/app/views/hyrax/compounds/_compound_row.html.erb index 63edb17b2a..4800e7177e 100644 --- a/app/views/hyrax/compounds/_compound_row.html.erb +++ b/app/views/hyrax/compounds/_compound_row.html.erb @@ -45,12 +45,27 @@ <% end %> <% case spec[:type] %> <% when 'controlled' %> + <%# A controlled member is a single-select by default. `multiple: + true` makes it a multi-select whose values submit as an array + (`[]` name suffix) and fan out into one entry per value + server-side. `autocomplete: true` tags the select so + compound_metadata.js binds a select2 typeahead; the full option + list is already in the DOM, so filtering is client-side. %> + <% multiple = spec[:multiple] %> + <% control_name = multiple ? "#{input_name}[]" : input_name %> <% select_classes = ['form-control', 'form-control-sm'] %> <% select_classes << 'force-select' if compound_subproperty_forced?(spec, value) %> - <%= select_tag input_name, + <% control_data = {} %> + <% if spec[:autocomplete] %> + <% control_data['hyrax-compound-controlled'] = true %> + <% control_data['placeholder'] = t('hyrax.compound_fields.search', default: 'Search') %> + <% end %> + <%= select_tag control_name, options_for_select(compound_subproperty_options(spec, value), value), id: input_id, - include_blank: true, + include_blank: !multiple, + multiple: multiple, + data: control_data, class: select_classes.join(' ') %> <% when 'url' %> <%= url_field_tag input_name, value, diff --git a/config/locales/hyrax.en.yml b/config/locales/hyrax.en.yml index e730b105f6..843f95febf 100644 --- a/config/locales/hyrax.en.yml +++ b/config/locales/hyrax.en.yml @@ -930,6 +930,7 @@ en: default_description: A User Collection can be created by any user to organize their works. compound_fields: add_row: Add %{label} + search: Search remove_row: Remove remove_row_with_label: Remove %{label} errors: diff --git a/documentation/compound_fields.md b/documentation/compound_fields.md index ba06f180f2..df831d0400 100644 --- a/documentation/compound_fields.md +++ b/documentation/compound_fields.md @@ -139,7 +139,7 @@ textarea). Supported `type:` values: | `url` | url input → auto-linked on show | The stored value is rendered as a clickable `` on show pages (matching the scalar `render_as: external_link` behavior). | | `work_or_url` | select2 typeahead → linked on show | Searches internal works (via the `compound_works` QA authority, backed by `Hyrax::CompoundWorkPickerBuilder`) **or** accepts a typed external URL. The stored value is a work id or a URL; on show, a work id links to the work's show page with its title (resolved by `Hyrax::CompoundWorkResolver`), a URL is auto-linked. | | `linked_record` | select2 typeahead → linked on show | A reference to a row in a database table (a person, organization, funder, place, …). The stored value is the row id; the picker searches that table and, optionally, lets a cataloger add a new record inline. On show, the id links to the record using the resolved label. The table and its procs are registered by the host application — see [`linked_record` sub-properties](#linked_record-sub-properties) below. | -| `controlled` | `` dropdown (optionally a select2 typeahead / multi-select) | Options come from either an inline `values:` list or a QA local authority named by `authority:` (see below). The row's stored value is preserved even if it is no longer offered, matching the `include_current_value` convention of the ordinary controlled-field partials. Two per-member `form:` opt-ins refine the input — see [Typeahead and multi-select](#typeahead-and-multi-select-for-controlled-members) below. | A `controlled` sub-property sources its options one of two ways: @@ -170,6 +170,36 @@ both are given, `values:` wins. Authority options are read through `Hyrax::TolerantSelectService`, so an authority file that omits the `active:` key behaves the same as it does for ordinary fields (terms default to active). +#### Typeahead and multi-select for `controlled` members + +A `controlled` member is a single-select plain `. select2 copies the control's `.form-control` +// classes onto its own container (a Bootstrap border + the fixed +// `.form-control-sm` height) while rendering its own bordered inner box (a +// "box within a box"), and the fixed height clips a multi-select's chosen pills. +// Flatten the inner box so only the form-control frame shows, and let that frame +// grow to fit the selected values instead of overflowing into the next field. +.hyrax-compound-row { + .select2-container.form-control { + height: auto; + min-height: calc(1.5em + 0.5rem + 2px); // matches .form-control-sm + padding: 0; + + .select2-choices, // multi-select + .select2-choice { // single-select + border: 0; + background: transparent; + box-shadow: none; + min-height: 0; + } + } + + // Multi-select: size the empty control to match a sibling .form-control-sm + // input (so Name and Role line up), and keep the chosen pills tidy as they + // wrap and grow the box. + .select2-container-multi.form-control { + .select2-choices { + padding: 0 0.5rem; + } + + .select2-search-field input.select2-input { + height: calc(1.5em + 0.5rem); + margin: 0; + line-height: 1.5; + } + + .select2-search-choice { + margin: 2px 0 2px 5px; + } + } +} From 3dae2246e0ee5b5280bae024205c0447802a8269 Mon Sep 17 00:00:00 2001 From: Shana Moore Date: Wed, 15 Jul 2026 15:03:33 -0700 Subject: [PATCH 3/7] Trim compound-field comments to explain why, not what Drop narration that restated the code and keep only non-obvious reasoning: the select2 v3 form-control quirk, the Reform populator detail, fan-out's cartesian product and nil-collapse edge case, why off-list values still render, and the html_safe precondition. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../javascripts/hyrax/compound_metadata.js | 12 ++++------ .../stylesheets/hyrax/_compound_metadata.scss | 16 +++++--------- .../concerns/hyrax/compound_field_behavior.rb | 22 +++++-------------- app/helpers/hyrax/compound_fields_helper.rb | 15 +++++-------- .../renderers/compound_attribute_renderer.rb | 6 ++--- app/services/hyrax/compound_schema.rb | 6 ++--- .../hyrax/compounds/_compound_row.html.erb | 9 +++----- 7 files changed, 28 insertions(+), 58 deletions(-) diff --git a/app/assets/javascripts/hyrax/compound_metadata.js b/app/assets/javascripts/hyrax/compound_metadata.js index 2b8ccf6734..2197b63b0c 100644 --- a/app/assets/javascripts/hyrax/compound_metadata.js +++ b/app/assets/javascripts/hyrax/compound_metadata.js @@ -67,12 +67,9 @@ }); } - // Bind select2 to a `controlled` sub-property tagged - // `data-hyrax-compound-controlled` (see _compound_row.html.erb). Unlike the - // work_or_url / linked_record pickers, this binds a NATIVE whose full option list is already in the DOM, so + // select2 filters client-side. function bindControlledSelects(root) { if (typeof jQuery === 'undefined' || !jQuery.fn.select2) return; jQuery(root).find('[data-hyrax-compound-controlled]').each(function() { @@ -287,8 +284,7 @@ } var html = template.innerHTML.replace(/__INDEX__/g, nextIndex); rowsHost.insertAdjacentHTML('beforeend', html); - // Bind select2 on any work_or_url / linked_record inputs and controlled - // typeahead selects in the new row. + // Cloned rows are fresh DOM, so re-run the select2 bindings. bindWorkOrUrlInputs(rowsHost.lastElementChild || rowsHost); bindControlledSelects(rowsHost.lastElementChild || rowsHost); section.dataset.nextIndex = String(nextIndex + 1); diff --git a/app/assets/stylesheets/hyrax/_compound_metadata.scss b/app/assets/stylesheets/hyrax/_compound_metadata.scss index 7102201296..d5e743ec52 100644 --- a/app/assets/stylesheets/hyrax/_compound_metadata.scss +++ b/app/assets/stylesheets/hyrax/_compound_metadata.scss @@ -89,13 +89,10 @@ } } -// A `controlled` sub-property with `form: { autocomplete: true }` binds select2 -// v3 over the native 's `.form-control` classes onto its container +// (Bootstrap border + fixed `.form-control-sm` height) while drawing its own +// bordered inner box, so a bound control shows a "box within a box" and clips +// its chosen pills. Flatten the inner box and let the frame grow to fit. .hyrax-compound-row { .select2-container.form-control { height: auto; @@ -111,9 +108,8 @@ } } - // Multi-select: size the empty control to match a sibling .form-control-sm - // input (so Name and Role line up), and keep the chosen pills tidy as they - // wrap and grow the box. + // Multi-select: match an empty control to a sibling .form-control-sm input so + // fields line up, and keep pills tidy as they wrap. .select2-container-multi.form-control { .select2-choices { padding: 0 0.5rem; diff --git a/app/forms/concerns/hyrax/compound_field_behavior.rb b/app/forms/concerns/hyrax/compound_field_behavior.rb index d4ee791ced..849e9b7763 100644 --- a/app/forms/concerns/hyrax/compound_field_behavior.rb +++ b/app/forms/concerns/hyrax/compound_field_behavior.rb @@ -55,10 +55,8 @@ def compound_field_names end # One populator serves every compound (Reform passes the property name as - # `as:`). Builds the replacement array of plain hashes — declared - # sub-property keys only, dropping `_destroy` and all-blank rows. A - # `multiple: true` member with several selected values fans its row out into - # one entry per value (see {#expand_multiple_members}). + # `as:`). A `multiple: true` member fans its row out into one entry per + # selected value (see {#expand_multiple_members}). def compound_attributes_populator(fragment:, as:, **_options) name = as.to_s.delete_suffix('_attributes') return unless respond_to?(name) @@ -76,16 +74,11 @@ def build_compound_rows(fragment, allowed_keys, multiple_keys = []) .compact end - # The sub-property keys declared `form: { multiple: true }` for this - # compound (empty when the definition is unavailable). def multiple_subproperty_keys(definition) return [] unless definition.is_a?(Hash) definition.fetch(:subproperties, {}).select { |_k, spec| spec[:multiple] }.keys end - # Returns nil for a row marked for destruction or whose declared - # sub-properties are all blank; otherwise an array of one or more persisted - # entry hashes — several when a `multiple` member carries multiple values. def compound_row_from(row, allowed_keys, multiple_keys = []) row = row_hash(row) return nil if %w[true 1].include?(row['_destroy'].to_s) @@ -96,19 +89,16 @@ def compound_row_from(row, allowed_keys, multiple_keys = []) expand_multiple_members(entry, multiple_keys) end - # A `multiple` member is coerced to a compacted array of stripped, non-blank - # values; any other member keeps its scalar (stripped if a string). def normalize_row_value(value, multiple) return Array(value).map { |v| v.is_a?(String) ? v.strip : v }.reject(&:blank?) if multiple value.is_a?(String) ? value.strip : value end - # Fan `multiple` members out so each stored entry holds one scalar value per - # such member. With a single multiple member (the common case) this yields - # one entry per selected value; with several it yields their cartesian - # product. A multiple member with no values collapses to a single nil, so a - # row whose only content is its other members is still stored once. + # Fan `multiple` members out into one entry per value (their cartesian + # product when a row has more than one). A `multiple` member with no values + # collapses to a single nil so a row carrying only its other members is + # still stored once. def expand_multiple_members(entry, multiple_keys) present = multiple_keys.select { |key| entry[key].is_a?(Array) && entry[key].any? } return [entry.transform_values { |v| v.is_a?(Array) ? nil : v }] if present.empty? diff --git a/app/helpers/hyrax/compound_fields_helper.rb b/app/helpers/hyrax/compound_fields_helper.rb index 480133f1cd..fc90ce8a43 100644 --- a/app/helpers/hyrax/compound_fields_helper.rb +++ b/app/helpers/hyrax/compound_fields_helper.rb @@ -65,11 +65,8 @@ def compound_schema_for(presenter) end ## - # Options for a `controlled` sub-property's ``. Any stored value not + # among the offered options is appended so a forced/stale value still renders. # # @return [Array] `[[label, id], ...]` def compound_subproperty_options(spec, current_value = nil) @@ -78,11 +75,9 @@ def compound_subproperty_options(spec, current_value = nil) end ## - # @return [Boolean] whether any present +current_value+ is not among the - # sub-property's offered options — i.e. a forced/stale value. The select - # gets the +force-select+ class in that case, matching the ordinary - # controlled-field convention. Handles an array value (a `multiple: true` - # member): forced when any selected value is off-list. + # @return [Boolean] whether any present +current_value+ is off the + # sub-property's offered options. Such a value gets the +force-select+ class + # so it still renders, matching the ordinary controlled-field convention. def compound_subproperty_forced?(spec, current_value = nil) values = Array(current_value).reject(&:blank?) return false if values.empty? diff --git a/app/renderers/hyrax/renderers/compound_attribute_renderer.rb b/app/renderers/hyrax/renderers/compound_attribute_renderer.rb index 047ab8a407..55a3127765 100644 --- a/app/renderers/hyrax/renderers/compound_attribute_renderer.rb +++ b/app/renderers/hyrax/renderers/compound_attribute_renderer.rb @@ -106,8 +106,7 @@ def linked_record_markup(sub_property, value) # stored value is itself a linkable URI (e.g. a rights-statement or license # URI), link the term to that URI — mirroring the ordinary rights/license # renderer. Non-URI controlled values (e.g. inline option ids) stay plain. - # An array value (a `multiple: true` member echoed back before fan-out) - # renders each term, comma-separated. + # An array can arrive when a `multiple` member is echoed back before fan-out. def controlled_markup(sub_property, value) return safe_join_terms(Array(value).map { |v| controlled_markup(sub_property, v) }) if value.is_a?(::Array) @@ -119,8 +118,7 @@ def controlled_markup(sub_property, value) end end - # Join already-escaped term fragments with a comma separator, preserving - # html_safe-ness. + # Fragments are already escaped, so joining and marking html_safe is safe. def safe_join_terms(fragments) fragments.map(&:to_s).join(', ').html_safe end diff --git a/app/services/hyrax/compound_schema.rb b/app/services/hyrax/compound_schema.rb index e777ea11a6..d2090127c6 100644 --- a/app/services/hyrax/compound_schema.rb +++ b/app/services/hyrax/compound_schema.rb @@ -263,10 +263,8 @@ def normalize_subproperty(opts) group: opts['group']&.to_s, cols: (form['cols'] || DEFAULT_COLS).to_i, as: form['as']&.to_s, - # `controlled`-only form opt-ins: `autocomplete` binds a select2 - # typeahead over the options; `multiple` allows several values, each - # fanned out into its own compound entry at submit time (see - # Hyrax::CompoundFieldBehavior). Both default false. + # `controlled`-only form opt-ins (both default false). `multiple` values + # fan out into one entry each at submit time (see Hyrax::CompoundFieldBehavior). autocomplete: truthy?(form['autocomplete']), multiple: truthy?(form['multiple']) }.merge(linked_record_keys(opts)) end diff --git a/app/views/hyrax/compounds/_compound_row.html.erb b/app/views/hyrax/compounds/_compound_row.html.erb index 4800e7177e..7c11974159 100644 --- a/app/views/hyrax/compounds/_compound_row.html.erb +++ b/app/views/hyrax/compounds/_compound_row.html.erb @@ -45,12 +45,9 @@ <% end %> <% case spec[:type] %> <% when 'controlled' %> - <%# A controlled member is a single-select by default. `multiple: - true` makes it a multi-select whose values submit as an array - (`[]` name suffix) and fan out into one entry per value - server-side. `autocomplete: true` tags the select so - compound_metadata.js binds a select2 typeahead; the full option - list is already in the DOM, so filtering is client-side. %> + <%# `multiple` submits values as an array (`[]` suffix) that fans out + into one entry per value server-side; `autocomplete` tags the + select for compound_metadata.js to bind a select2 typeahead. %> <% multiple = spec[:multiple] %> <% control_name = multiple ? "#{input_name}[]" : input_name %> <% select_classes = ['form-control', 'form-control-sm'] %> From bb2b0fa7e478d73ab394d7ad89566e40e414d834 Mon Sep 17 00:00:00 2001 From: Shana Moore Date: Wed, 15 Jul 2026 15:14:47 -0700 Subject: [PATCH 4/7] Address review feedback on compound typeahead styling and fan-out Scope the select2 styling to a container class tagged by bindControlledSelects so it no longer applies to other select2 controls (work_or_url / linked_record) in the same compound row. Fix scss-lint nits: property order, drop the element-qualified selector, and avoid the 4-value margin shorthand; also match the empty multi-select's height to a sibling form-control-sm input. expand_multiple_members now only collapses `multiple` members to nil, so it never wipes other members that happen to arrive as arrays and never leaks a `[]` for an unselected `multiple` member. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../javascripts/hyrax/compound_metadata.js | 3 ++ .../stylesheets/hyrax/_compound_metadata.scss | 52 +++++++++---------- .../concerns/hyrax/compound_field_behavior.rb | 7 ++- 3 files changed, 32 insertions(+), 30 deletions(-) diff --git a/app/assets/javascripts/hyrax/compound_metadata.js b/app/assets/javascripts/hyrax/compound_metadata.js index 2197b63b0c..a8c27952b2 100644 --- a/app/assets/javascripts/hyrax/compound_metadata.js +++ b/app/assets/javascripts/hyrax/compound_metadata.js @@ -84,6 +84,9 @@ allowClear: !$el.prop('multiple'), placeholder: $el.data('placeholder') || '' }); + // Tag the generated container so the styling stays scoped to these + // selects and off other select2 controls in the row. + $el.select2('container').addClass('hyrax-compound-controlled-select2'); }); } diff --git a/app/assets/stylesheets/hyrax/_compound_metadata.scss b/app/assets/stylesheets/hyrax/_compound_metadata.scss index d5e743ec52..e26cb464de 100644 --- a/app/assets/stylesheets/hyrax/_compound_metadata.scss +++ b/app/assets/stylesheets/hyrax/_compound_metadata.scss @@ -93,36 +93,32 @@ // (Bootstrap border + fixed `.form-control-sm` height) while drawing its own // bordered inner box, so a bound control shows a "box within a box" and clips // its chosen pills. Flatten the inner box and let the frame grow to fit. -.hyrax-compound-row { - .select2-container.form-control { - height: auto; - min-height: calc(1.5em + 0.5rem + 2px); // matches .form-control-sm - padding: 0; - - .select2-choices, // multi-select - .select2-choice { // single-select - border: 0; - background: transparent; - box-shadow: none; - min-height: 0; - } +// (bindControlledSelects tags the container so this stays off other select2 +// controls in the row.) +.hyrax-compound-controlled-select2.form-control { + height: auto; + min-height: calc(1.5em + 0.5rem + 2px); // matches .form-control-sm + padding: 0; + + .select2-choice, // single-select + .select2-choices { // multi-select + background: transparent; + border: 0; + box-shadow: none; + min-height: 0; } +} - // Multi-select: match an empty control to a sibling .form-control-sm input so - // fields line up, and keep pills tidy as they wrap. - .select2-container-multi.form-control { - .select2-choices { - padding: 0 0.5rem; - } - - .select2-search-field input.select2-input { - height: calc(1.5em + 0.5rem); - margin: 0; - line-height: 1.5; - } +// Multi-select: match an empty control to a sibling .form-control-sm input so +// fields line up, and keep pills tidy as they wrap. +.select2-container-multi.hyrax-compound-controlled-select2 { + .select2-choices { + padding: 0 0.5rem; + } - .select2-search-choice { - margin: 2px 0 2px 5px; - } + .select2-search-field .select2-input { + height: calc(1.5em + 0.5rem); + line-height: 1.5; + margin: 0; } } diff --git a/app/forms/concerns/hyrax/compound_field_behavior.rb b/app/forms/concerns/hyrax/compound_field_behavior.rb index 849e9b7763..c8881cc37c 100644 --- a/app/forms/concerns/hyrax/compound_field_behavior.rb +++ b/app/forms/concerns/hyrax/compound_field_behavior.rb @@ -101,11 +101,14 @@ def normalize_row_value(value, multiple) # still stored once. def expand_multiple_members(entry, multiple_keys) present = multiple_keys.select { |key| entry[key].is_a?(Array) && entry[key].any? } - return [entry.transform_values { |v| v.is_a?(Array) ? nil : v }] if present.empty? + # Only `multiple` members are collapsed (to nil), so a non-selected one + # never leaks a `[]` and other members keep whatever they came in as. + base = entry.merge((multiple_keys - present).index_with(nil)) + return [base] if present.empty? value_lists = present.map { |key| entry[key] } value_lists.first.product(*value_lists[1..]).map do |combo| - entry.merge(present.zip(combo).to_h) + base.merge(present.zip(combo).to_h) end end end From 7f5925bb2572fadd855384b5abc58e77e40b4bb1 Mon Sep 17 00:00:00 2001 From: Shana Moore Date: Wed, 15 Jul 2026 15:22:36 -0700 Subject: [PATCH 5/7] Flatten box-in-box for all compound-row select2 controls Extend the select2 form-control flattening to every select2 control in a compound row (Item / work_or_url, linked_record, and the new controlled typeahead), bounded to `.hyrax-compound-row`, so the whole compound form reads consistently instead of the work_or_url pickers keeping a nested border. This drops the per-container marker class in favor of the row-scoped selector. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../javascripts/hyrax/compound_metadata.js | 3 --- .../stylesheets/hyrax/_compound_metadata.scss | 21 ++++++++++++------- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/app/assets/javascripts/hyrax/compound_metadata.js b/app/assets/javascripts/hyrax/compound_metadata.js index a8c27952b2..2197b63b0c 100644 --- a/app/assets/javascripts/hyrax/compound_metadata.js +++ b/app/assets/javascripts/hyrax/compound_metadata.js @@ -84,9 +84,6 @@ allowClear: !$el.prop('multiple'), placeholder: $el.data('placeholder') || '' }); - // Tag the generated container so the styling stays scoped to these - // selects and off other select2 controls in the row. - $el.select2('container').addClass('hyrax-compound-controlled-select2'); }); } diff --git a/app/assets/stylesheets/hyrax/_compound_metadata.scss b/app/assets/stylesheets/hyrax/_compound_metadata.scss index e26cb464de..ae70d13daa 100644 --- a/app/assets/stylesheets/hyrax/_compound_metadata.scss +++ b/app/assets/stylesheets/hyrax/_compound_metadata.scss @@ -89,13 +89,12 @@ } } -// select2 v3 copies the whose full option list is already in the DOM, so - // select2 filters client-side. + // The full option list is already in the DOM, so select2 filters + // client-side (no ajax, unlike the work_or_url / linked_record pickers). function bindControlledSelects(root) { if (typeof jQuery === 'undefined' || !jQuery.fn.select2) return; jQuery(root).find('[data-hyrax-compound-controlled]').each(function() { @@ -78,9 +77,7 @@ $el.select2({ width: '100%', - // A single select carries a blank option (include_blank), so - // allowClear gives it an "x"; a multiple has none and needs a - // placeholder to prompt. + // allowClear only on single-selects; a v3 multi warns without a placeholder. allowClear: !$el.prop('multiple'), placeholder: $el.data('placeholder') || '' }); diff --git a/app/assets/stylesheets/hyrax/_compound_metadata.scss b/app/assets/stylesheets/hyrax/_compound_metadata.scss index ae70d13daa..ea5f418e01 100644 --- a/app/assets/stylesheets/hyrax/_compound_metadata.scss +++ b/app/assets/stylesheets/hyrax/_compound_metadata.scss @@ -89,40 +89,35 @@ } } -// select2 v3 copies the control's `.form-control` classes onto its container -// (Bootstrap border + fixed `.form-control-sm` height) and then draws its own -// bordered inner box, so every select2 control in a compound row shows a -// "box within a box". Flatten the inner box so only the form-control frame -// shows and let the frame grow to fit its contents. +// select2 v3 copies the control's `.form-control` classes onto its container and +// then draws its own bordered inner box, so a bound control renders a "box within +// a box". Flatten the inner box so only the form-control frame shows. .hyrax-compound-row .select2-container.form-control { height: auto; min-height: calc(1.5em + 0.5rem + 2px); // matches .form-control-sm padding: 0; - .select2-choice, // single-select - .select2-choices { // multi-select + .select2-choice, + .select2-choices { background: transparent; border: 0; box-shadow: none; min-height: 0; } - // The single-select dropdown arrow otherwise sits in its own bordered cell. .select2-arrow { background: transparent; border-left: 0; } } -// Multi-select: match an empty control to a sibling .form-control-sm input so -// fields line up, and keep pills tidy as they wrap. .hyrax-compound-row .select2-container-multi.form-control { .select2-choices { padding: 0 0.5rem; } .select2-search-field .select2-input { - height: calc(1.5em + 0.5rem); + height: calc(1.5em + 0.5rem); // matches .form-control-sm line-height: 1.5; margin: 0; } diff --git a/app/forms/concerns/hyrax/compound_field_behavior.rb b/app/forms/concerns/hyrax/compound_field_behavior.rb index c8881cc37c..dd17d6b5a1 100644 --- a/app/forms/concerns/hyrax/compound_field_behavior.rb +++ b/app/forms/concerns/hyrax/compound_field_behavior.rb @@ -54,9 +54,6 @@ def compound_field_names [] end - # One populator serves every compound (Reform passes the property name as - # `as:`). A `multiple: true` member fans its row out into one entry per - # selected value (see {#expand_multiple_members}). def compound_attributes_populator(fragment:, as:, **_options) name = as.to_s.delete_suffix('_attributes') return unless respond_to?(name) @@ -95,14 +92,11 @@ def normalize_row_value(value, multiple) value.is_a?(String) ? value.strip : value end - # Fan `multiple` members out into one entry per value (their cartesian - # product when a row has more than one). A `multiple` member with no values - # collapses to a single nil so a row carrying only its other members is - # still stored once. + # A `multiple` member with no selection collapses to nil, not an empty array, + # so a row carrying only its other members is still stored once. Only + # `multiple` members are touched, so other members are never clobbered. def expand_multiple_members(entry, multiple_keys) present = multiple_keys.select { |key| entry[key].is_a?(Array) && entry[key].any? } - # Only `multiple` members are collapsed (to nil), so a non-selected one - # never leaks a `[]` and other members keep whatever they came in as. base = entry.merge((multiple_keys - present).index_with(nil)) return [base] if present.empty? diff --git a/app/helpers/hyrax/compound_fields_helper.rb b/app/helpers/hyrax/compound_fields_helper.rb index fc90ce8a43..a24b47f9a1 100644 --- a/app/helpers/hyrax/compound_fields_helper.rb +++ b/app/helpers/hyrax/compound_fields_helper.rb @@ -65,8 +65,7 @@ def compound_schema_for(presenter) end ## - # Options for a `controlled` sub-property's `` for a `controlled` sub-property, applying the + # `multiple` (array name, no blank) and `autocomplete` (select2 typeahead) opt-ins. + def compound_controlled_select(spec, value, input_name, input_id) + multiple = spec[:multiple] + classes = ['form-control', 'form-control-sm'] + classes << 'force-select' if compound_subproperty_forced?(spec, value) + select_tag(multiple ? "#{input_name}[]" : input_name, + options_for_select(compound_subproperty_options(spec, value), value), + id: input_id, + include_blank: !multiple, + multiple: multiple, + data: compound_controlled_data(spec), + class: classes.join(' ')) + end + + # select2 typeahead data attrs for an `autocomplete` sub-property, else empty. + def compound_controlled_data(spec) + return {} unless spec[:autocomplete] + + { 'hyrax-compound-controlled' => true, + 'placeholder' => t('hyrax.compound_fields.search', default: 'Search') } + end + ## # The pre-selected `[label, value]` option for a `work_or_url` sub-property's # select2, or nil when empty. An internal work id resolves to its title; an diff --git a/app/views/hyrax/compounds/_compound_row.html.erb b/app/views/hyrax/compounds/_compound_row.html.erb index 7e4625fa1d..963373bacc 100644 --- a/app/views/hyrax/compounds/_compound_row.html.erb +++ b/app/views/hyrax/compounds/_compound_row.html.erb @@ -45,24 +45,7 @@ <% end %> <% case spec[:type] %> <% when 'controlled' %> - <%# `multiple` submits an array (`[]` suffix) that fans out server-side; - `autocomplete` tags the select for compound_metadata.js. %> - <% multiple = spec[:multiple] %> - <% control_name = multiple ? "#{input_name}[]" : input_name %> - <% select_classes = ['form-control', 'form-control-sm'] %> - <% select_classes << 'force-select' if compound_subproperty_forced?(spec, value) %> - <% control_data = {} %> - <% if spec[:autocomplete] %> - <% control_data['hyrax-compound-controlled'] = true %> - <% control_data['placeholder'] = t('hyrax.compound_fields.search', default: 'Search') %> - <% end %> - <%= select_tag control_name, - options_for_select(compound_subproperty_options(spec, value), value), - id: input_id, - include_blank: !multiple, - multiple: multiple, - data: control_data, - class: select_classes.join(' ') %> + <%= compound_controlled_select(spec, value, input_name, input_id) %> <% when 'url' %> <%= url_field_tag input_name, value, id: input_id, diff --git a/spec/helpers/hyrax/compound_fields_helper_spec.rb b/spec/helpers/hyrax/compound_fields_helper_spec.rb index 1892f510ba..eff07c7646 100644 --- a/spec/helpers/hyrax/compound_fields_helper_spec.rb +++ b/spec/helpers/hyrax/compound_fields_helper_spec.rb @@ -64,6 +64,44 @@ end end + describe '#compound_controlled_select' do + let(:value) { nil } + let(:spec) { { type: 'controlled', authority: nil, values: [%w[Author Author], %w[Editor ed]] } } + subject(:node) { Capybara.string(helper.compound_controlled_select(spec, value, 'work[roles]', 'work_roles')) } + + it 'renders a single-select with a blank option and no typeahead tag by default' do + expect(node).to have_selector("select[name='work[roles]'][id='work_roles']") + expect(node).not_to have_selector('select[multiple]') + expect(node).to have_selector("select option[value='']", visible: :all) + expect(node).not_to have_selector('select[data-hyrax-compound-controlled]') + end + + context 'when multiple' do + let(:spec) { { type: 'controlled', authority: nil, values: [%w[Author Author]], multiple: true } } + + it 'uses an array name, is multiple, and drops the blank option' do + expect(node).to have_selector("select[name='work[roles][]'][multiple]") + expect(node).not_to have_selector("select option[value='']", visible: :all) + end + end + + context 'when autocomplete' do + let(:spec) { { type: 'controlled', authority: nil, values: [%w[Author Author]], autocomplete: true } } + + it 'tags the select for the select2 typeahead with a placeholder' do + expect(node).to have_selector('select[data-hyrax-compound-controlled][data-placeholder]') + end + end + + context 'with an off-list stored value' do + let(:value) { 'Legacy' } + + it 'adds the force-select class' do + expect(node).to have_selector('select.force-select') + end + end + end + describe '#compound_subproperty_label' do it 'falls back to a humanized sub-property key when no translation exists' do expect(helper.compound_subproperty_label(:nonexistent_compound, :some_field)).to eq('Some field')