Skip to content

Fix person form UX issues#1306

Open
fbacall wants to merge 2 commits into
masterfrom
fix-people-form
Open

Fix person form UX issues#1306
fbacall wants to merge 2 commits into
masterfrom
fix-people-form

Conversation

@fbacall
Copy link
Copy Markdown
Member

@fbacall fbacall commented May 12, 2026

Summary of changes

Motivation and context

See above.

Checklist

  • I have read and followed the CONTRIBUTING guide.
  • I confirm that I have the authority necessary to make this contribution on behalf of its copyright owner and agree to license it to the TeSS codebase under the BSD license.

- Fix indexes for unsaved people. Fixes #1295
- Ensure delete button works on newly-added people. Fixes #1304
- Ensure autocomplete works on existing people. Fixes #1305
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves the “people” nested form UX for authors/contributors/etc by ensuring dynamically added entries behave correctly across validation errors, can be removed reliably, and have autocomplete bound consistently.

Changes:

  • Render stable (non-replace-me) indices for unsaved people rows.
  • Refactor People JS to centralize autocomplete binding and apply it to existing rows.
  • Bind delete behavior via delegated events so it works for newly added rows.

Reviewed changes

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

File Description
app/views/common/_people_form.html.erb Updates the index used for person sub-forms to avoid replace-me for unsaved entries.
app/assets/javascripts/people.js Refactors binding to fix autocomplete on existing rows and deletion on newly added rows.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 23 to 26
<% f.object.send(field_name).each_with_index do |person, index| %>
<%= render partial: 'common/person_form',
locals: { field_prefix: "#{model_name}[#{field_name}]", index: person.id, person: person, role: role } %>
locals: { field_prefix: "#{model_name}[#{field_name}]", index: person.id || (index + 1), person: person, role: role } %>
<% end %>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants