theunitedeffort / theunitedeffort.org

The United Effort Organization website
https://theunitedeffort.org
3 stars 5 forks source link

Add checkbox for users to explicitly say they have no assets. #397

Closed trevorshannon closed 1 year ago

trevorshannon commented 1 year ago

Similar to the Income sub-section, we should have a checkbox in the Assets sub-section that users can check if they have zero assets. When that box is checked, the interactive elements to add assets should be deactivated, and any assets already added should be removed or zeroed out.

DairyProducts commented 1 year ago

Assets do not seem to be assigned unique IDs for each one added. How should this be done?

image

I also can't seem to identify where the logic for when "remove" is clicked is defined.

trevorshannon commented 1 year ago

The individual Value fields do have unique IDs (e.g. income-assets-member1-0). The container <div>s do not. You can use query selectors to get those if you need. Here are some examples, depending on which container element you need:

document.querySelectorAll('#page-income-assets ul.dynamic_field_list');
document.querySelectorAll('#page-income-assets fieldset');
document.querySelectorAll('#page-income-assets div.dynamic_field_list_wrapper');

Here's one idea to achieve the goal:

When the user clicks on some new checkbox to say they have no assets, handle that event with a function that removes all items from all assets lists (there may be more than one list since each household member can have assets) and disables the "Enter an asset for this person" buttons. There is a function called removeDynamicFieldListItem that could be helpful, and I think resetDynamicFieldLists might be even more helpful. I'm pretty sure that just removing the entries should be enough; no need to explicitly zero each one out or anything.

I feel like the new checkbox should be near the bottom so that it's closer to the Next button, but I'm open to other ideas.

trevorshannon commented 1 year ago

It would be nice to have this functionality in place when we release this tool to the public at the end of June, but not strictly necessary. @DairyProducts how are you feeling about this issue?

DairyProducts commented 1 year ago

My apologies for not replying or taking action sooner. I've been out of the country since early June and unable to access my computer. I'll be back next week; I will work on this as soon as possible.

trevorshannon commented 1 year ago

No problem at all! I'll move this to the next release milestone and you can work on it whenever you have time. Enjoy your time away from home.

DairyProducts commented 1 year ago

I'm not sure how the schema for setting up listeners for elements works. In the latest commit to the branch I've attempted to set up a listener on the new checkbox but it appears to not be working (tested with console.log function in onNoAssetCheck). Could you please take a look and let me know if something should be changed? Thanks.

trevorshannon commented 1 year ago

It looks like the click listener is working to me, nice job! I checked out your eligibility/no-asset-checkbox branch and I see pass in the console when I click your new checkbox in Chrome 114.

One thing that may cause an issue (depends on the browser): the id of your <ul> is hh-member-types which was already used in the document, and ids need to be unique.

I also noticed your feature branch is pretty far behind main. It could be that main has advanced since you made that branch, or it could be that you had not done a git pull before creating the local branch off main. When you merge main into eligibility/no-asset-checkbox (which I suggest you do sooner rather than later in order to get your feature branch up to date with main) there will be some merge conflicts, which are always a pain. Let me know if you need any help resolving those.

You can merge main in to your branch with the GitHub Desktop client or with:

git checkout main
git pull
git checkout eligibility/no-asset-checkbox
git merge main

by the way, after you do that merge, you'll find the eligibility tool at /benefits-eligibility instead of /public-assistance/eligibility

DairyProducts commented 1 year ago

I've merged main into the branch. The listener still isn't working on my end on either Firefox 116 or Chrome 115. I don't know if I'm looking in the right place; I'm using the console in the inspect menu on both browsers. I'm also using the environment produced by npm run dev to test this feature.

trevorshannon commented 1 year ago

Hmm, I'm not exactly sure of the problem. I normally use netlify dev to start the server, but I imagine it does not really matter how the files are served, as long as the HTML and JS gets to the browser. It is, however, a difference between our setups.

You can see if your checkbox element has the listener attached in the dev tools Elements tab:

Screenshot 2023-08-04 at 12 39 45 PM

It sounds like you are using the correct console. Here's mine:

Screenshot 2023-08-04 at 12 38 17 PM

Some other debugging ideas:

trevorshannon commented 1 year ago

Assigning to me per a separate email discussion.