-
Notifications
You must be signed in to change notification settings - Fork 19
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
#2311: New ComboBox widget for form fields [BOB] #3302
base: main
Are you sure you want to change the base?
Conversation
…in suborganization form to use new combobox widget
🥳 Successfully deployed to developer sandbox bob. |
🥳 Successfully deployed to developer sandbox bob. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Line ending in one file
@dave-kennedy-ecs I'm not seeing the combo box applied on the state, territory, and military post dropdowns (last AC). |
Correction, I am seeing it on the non-org model domain request, but it wasn't on the organization page on org model or domain management organization page (non-org model). |
🥳 Successfully deployed to developer sandbox bob. |
🥳 Successfully deployed to developer sandbox bob. |
@gabydisarli thank you for catching this. I fixed this bug. It should now work as before. |
🥳 Successfully deployed to developer sandbox bob. |
@gabydisarli I fixed these as well. Sorry, I was having trouble initially understanding the ACs and missed a few selects that I was supposed to change to comboboxes. I think I now have all the requested cases as comboboxes rather than selects. |
🥳 Successfully deployed to developer sandbox bob. |
🥳 Successfully deployed to developer sandbox bob. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like the requesting entity page (portfolio domain request) is throwing a js error! The "Add suborganization information" box should be hidden on page load for new requests.
By default, this page adds a 'fake' option called "other" to this dropdown to track if they are requesting a new suborganization (#2771). Its likely failing on that part of it
🥳 Successfully deployed to developer sandbox bob. |
Done |
This bug is now fixed |
🥳 Successfully deployed to developer sandbox bob. |
1 similar comment
🥳 Successfully deployed to developer sandbox bob. |
🥳 Successfully deployed to developer sandbox bob. |
Two additional observations:
|
🥳 Successfully deployed to developer sandbox bob. |
Thanks @zandercymatics I'll take a look at those items you pointed out when the PR is ready to review again |
🥳 Successfully deployed to developer sandbox bob. |
🥳 Successfully deployed to developer sandbox bob. |
1 similar comment
🥳 Successfully deployed to developer sandbox bob. |
// Add fake "other" option to sub_organization select | ||
if (select && !Array.from(select.options).some(option => option.value === "other")) { | ||
select.add(new Option(subOrgCreateNewOption, "other")); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Handling of 'other' has been moved from javascript to the form
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work. One question, though
is_requesting_new_suborganization = False | ||
if self.initial and "is_requesting_new_suborganization" in self.initial: | ||
# Call the method if it exists | ||
is_requesting_new_suborganization = self.initial["is_requesting_new_suborganization"]() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This works, but why doesn't it throw a syntax error? Since is_requesting_new_suborganization
is a field on the form. I know it is a function, too, though - so maybe some magic is going over my head here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is_requesting_new_suborganization is a field in the form. The form initialization, specifically get_initial_for_field, sets the field to the callable method on the (existing) object. this is how it can be called on line 152. note that this line is only called during a GET, not during a POST, so there isn't a conflict between the input submitted from a form, and the value initialized from the db object
Ticket
Resolves #2311
Changes
Context for reviewers
Setup
These options on the different forms are not always visible and depends on waffle flag settings. To test Domain Request Organization form (federal agency and state/territory combo boxes), I had to set organization_feature waffle flag to False. To test Domain Suborganization form (suborganization combo box), I had to set organization_feature waffle flag to True.
Code Review Verification Steps
As the original developer, I have
Satisfied acceptance criteria and met development standards
Ensured code standards are met (Original Developer)
Validated user-facing changes (if applicable)
As a code reviewer, I have
Reviewed, tested, and left feedback about the changes
Validated user-facing changes as a developer
Note: Multiple code reviewers can share the checklists above, a second reviewer should not make a duplicate checklist. All checks should be checked before approving, even those labeled N/A.
As a designer reviewer, I have
Verified that the changes match the design intention
Validated user-facing changes as a designer
References
Screenshots
Domain Request Organization form with Federal Agency and State combo boxes:
Domain Suborganization form: