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

Fix classes for counts of result types #8540

Merged
merged 2 commits into from
Aug 22, 2024
Merged

Fix classes for counts of result types #8540

merged 2 commits into from
Aug 22, 2024

Conversation

wpears
Copy link
Member

@wpears wpears commented Aug 21, 2024

Closes https://github.local/CFGOV/platform/issues/4589
This got introduced in #8274

rout does some kinda nasty string concatenation to build up selectors that would have been missed in find/replace.

This fixes counts for me:
Screenshot 2024-08-21 at 2 11 23 PM

@wpears wpears requested a review from anselmbradford August 21, 2024 21:12
@anselmbradford
Copy link
Member

What do you think about adding a small utility imported into https://github.com/cfpb/consumerfinance.gov/blob/main/cfgov/unprocessed/apps/rural-or-underserved-tool/js/common.js that converts result.type from camelCase to kebab-case? Then the classes could be kebab and consistent with the rest of the CSS.

@wpears
Copy link
Member Author

wpears commented Aug 22, 2024

I'd kinda prefer to leave this as is. Feels like a little speedbump to me that something is different and some wonky JS is touching the classes.

@anselmbradford
Copy link
Member

Are the notRuralCase and notFoundCase classes even used?

@wpears
Copy link
Member Author

wpears commented Aug 22, 2024

Are the notRuralCase and notFoundCase classes even used?

yeah, at

changeElText('.' + type + 'Case', noun + ' ' + verb);

@wpears wpears added this pull request to the merge queue Aug 22, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 22, 2024
@wpears wpears added this pull request to the merge queue Aug 22, 2024
Merged via the queue into main with commit 2cf26d8 Aug 22, 2024
17 checks passed
@wpears wpears deleted the rout-count-bug branch August 22, 2024 18:42
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.

2 participants