Skip to content
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

TDP: semi-terrible fix for activities search bug #7659

Merged
merged 4 commits into from
Apr 13, 2023
Merged

Conversation

anselmbradford
Copy link
Member

@anselmbradford anselmbradford commented Apr 13, 2023

TDP has an existing bug in production

Screen Shot 2023-04-13 at 12 26 16 PM

This PR performs a semi-terrible solution of copying the older expandable JS code into TDP, in order to fix the bug, while isolating it from the current design-system expandable code.

Additions

  • Add a copy of the expandable template and JS into TDP.

Changes

How to test this PR

  1. yarn build and check http://localhost:8000/consumer-tools/educator-tools/youth-financial-education/teach/activities/?q= vs production and see that when the expandable facets are toggled they don't overlap themselves.

Notes and todos

  • The correct long-term solution for this is to develop out a component for the expandable-facets and the nested inputs, which then gets used in TDP, prepaid agreements, and CCDB. This work has already gotten underway in Add input tree and expandable facets design-system#1628, however, the TDP nested checkboxes have the behavior of allowing checking of the children independent of checking the parent. So checking the parent does not equal checking all the children. Also, TDP uses link styling for the parents, while CCDB uses an underline-less hover that has inaccessible color contrast. These are issues that we don't want to enshrine in the design-system. So, the components should be built out in the DS in a proper way, and then TDP should be updated to use those new components.

…rm/activity_search_facets_and_results.html

Co-authored-by: Andy Chosak <andy.chosak@cfpb.gov>
Copy link
Member

@wpears wpears left a comment

Choose a reason for hiding this comment

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

This is a fine approach. TDP does many things in its own way, so sealing that off from affecting other work in the short/medium term seems great.

@anselmbradford anselmbradford added this pull request to the merge queue Apr 13, 2023
Merged via the queue into main with commit df74323 Apr 13, 2023
@anselmbradford anselmbradford deleted the ans_fix_tdp branch April 13, 2023 18:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants