feat: application side nav

ColinBuyck commented 3 months ago

This PR addresses https://github.com/metrotranscom/doorway/issues/437

[x] Addresses the issue in full
[ ] Addresses only certain aspects of the issue

Description

This PR utilizes the tab component so a user can filter the applications they would like to see.

Note One: This implementation results in applications being re-fetched each time they select a new category. Based on the performance of this, I am wondering if there is a more reduced fetch call that only provides the required information.

Note Two: I made some assumptions on the mobile experience of this section, let me know what y'all think!

Note Three: I was looking into expanding the test coverage and was considering ctypress tests but know we have been cautious in adding more cypress. Additionally, seeing how limited my-applications.spec.ts is would it be worth it to expand it? Planning on expending integration test coverage in #4053

How Can This Be Tested/Reviewed?

This can be tested by signing into the public site and first testing the empty states of the different sections. If there are no applications at all, the message will be to browse more listings but if you have applied but none of the applications are in the selected state, it will be a custom message. Then apply to multiple listings. Lastly, change the status of those listings and observe the applications move to the corresponding section

Author Checklist:

[x] Added QA notes to the issue with applicable URLs
[x] Reviewed in a desktop view
[x] Reviewed in a mobile view
[x] Reviewed considering accessibility
[ ] Added tests covering the changes
[ ] Made corresponding changes to the documentation
[ ] Ran yarn generate:client and/or created a migration when required

Review Process:

Read and understand the issue
Ensure the author has added QA notes
Review the code itself from a style point of view
Pull the changes down locally and test that the acceptance criteria is met
Either (1) explicitly ask a clarifying question, (2) request changes, or (3) approve the PR, even if there are very small remaining changes, if you don't need to re-review after the updates

netlify[bot] commented 3 months ago

Deploy Preview for partners-bloom-dev ready!

Name	Link
Latest commit	f0a1eadd58eea4255246dc092b0af901e182eb0c
Latest deploy log	https://app.netlify.com/sites/partners-bloom-dev/deploys/66ba592fd05f630008310d54
Deploy Preview	https://deploy-preview-4237--partners-bloom-dev.netlify.app
Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

netlify[bot] commented 3 months ago

Deploy Preview for bloom-exygy-dev ready!

Name	Link
Latest commit	f0a1eadd58eea4255246dc092b0af901e182eb0c
Latest deploy log	https://app.netlify.com/sites/bloom-exygy-dev/deploys/66ba592fe8696d000820178c
Deploy Preview	https://deploy-preview-4237--bloom-exygy-dev.netlify.app
Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

emilyjablonski commented 3 months ago

Not sure if it matters, but in the lottery designs, the order of the last two tabs is switched, the design goes all --> lottery run --> applications closed --> accepting applications

emilyjablonski commented 3 months ago

When I test this in the deploy preview, I relatively quickly am getting the issue where the listing data stops appearing, and it looks like what is happening is that I'm getting throttled. We may need to update the threshold. I'm interested in separately moving forward w a smaller view, created a ticket here 🏹

emilyjablonski commented 3 months ago

Seeing this button when I am already on that page felt confusing, we maybe just need to remove it from that view.

ColinBuyck commented 3 months ago

Thanks for the review @emilyjablonski ! I've fixed most of these but just have a couple notes. I changed the throttle envs to match hba prod but would like to bring it up to the team on Monday to discuss what value seems right since I'm still seeing it throttled with the new settings. I'm a little worried about undermining security (if the throttle limit extends to prod) based on this one feature. Secondly, I explored the view thing we discussed and should've noted it in the description but my understanding is that we're using views to make complex queries (ie. joins) easier rather than get a subset of fields in a single table. For example, "fundamentals" view still returns all the fields below

Might still be worth it to do a custom query like getPublicUserEmailInfo() if we think the number of fields is driving the slowdown. Alternatively if its the number of calls on the db we could batch the ids and then make a single call. Orrr explore using context

ColinBuyck commented 3 months ago

@emilyjablonski Also updating to 1 review needed since some of this logic is altered for the next lottery FE ticket. EDIT: I take that back. Since the throttling/querying approach is proving to be more of an issue, I don't think we should rush this piece.

bloom-housing / bloom