Add contributor affiliation updates to work form #1659
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
aaron-collier
force-pushed
the
t329-author-affiliations
branch
3 times, most recently
from
9eb97c2 to
3251387
Compare
aaron-collier
force-pushed
the
t329-author-affiliations
branch
7 times, most recently
from
83e0803 to
5a700c5
Compare
aaron-collier
changed the title
| <div class="row"> | ||
| <div class="col"> | ||
| <div data-controller="invalid-feedback" data-invalid-feedback-tab-error-outlet=".tab-error" data-invalid-feedback-tab-value="affiliations"> | ||
| <%= render Elements::Forms::InvalidFeedbackComponent.new(form:, field_name: :uri, data: { invalid_feedback_target: 'feedback' }, classes: 'mb-2') %> |
There was a problem hiding this comment.
Why invalid feedback on the URI and not the institution?
There was a problem hiding this comment.
My thinking was that institution being populated doesn't necessarily mean they properly selected a value from the ROR return. Though, for the UI - it may be the best place for it. I'll take a pass at it.
| @@ -19,7 +19,7 @@ export default class extends Controller { | |||
| if (!this.hasContainerTarget) return | |||
| this.containerTarget.querySelectorAll('.form-instance').forEach((instanceEl, index) => { | |||
| instanceEl.querySelectorAll('.move-up').forEach((btnEl) => btnEl.classList.toggle('d-none', index === 0)) | |||
| instanceEl.querySelectorAll('.move-down').forEach((btnEl) => btnEl.classList.toggle('d-none', index === this.instanceTargets.length - 1)) | |||
| instanceEl.querySelectorAll('.move-down').forEach((btnEl) => btnEl.classList.toggle('d-none', index === this.instanceTargets.length - 2)) | |||
There was a problem hiding this comment.
What is the reason for this change?
There was a problem hiding this comment.
There are now 2 forms for every person type contributor, so currently the -1 is what hides the move up/move down buttons when there is only a single entry. This doesn't work now with a person contributor as there are 2 forms, so we have to -2 here.
aaron-collier
force-pushed
the
t329-author-affiliations
branch
2 times, most recently
from
f8091fa to
3259fdd
Compare
justinlittman
force-pushed
the
t329-author-affiliations
branch
2 times, most recently
from
27a5f86 to
fa2e473
Compare
justinlittman
force-pushed
the
t329-author-affiliations
branch
from
48ef57d to
73485bc
Compare
justinlittman
approved these changes
Jul 22, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Taking this out of draft for review.
Fixes #329
(cc: @justinlittman) I think this is ready for review with 2 notable styling issue that I could use help with:
I can't seem to get the trashcan icon to the right of the affiliation form:
I'm wondering how to go about changing this validation message to something useful. The validation is actually based on the hidden URI field and happens if the user enters a value but does not select from the pull down to get the ROR URI, therefore causing a validation error for the affiliation URI. I'm thinking it should say something like "please select a valid institution from the drop down" or something.