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 Activity Search: Mobile customization #6427

Merged
merged 9 commits into from
May 13, 2021
Merged

Conversation

karenkitchens
Copy link

@karenkitchens karenkitchens commented May 4, 2021

TDP activity search need to be customized since the desktop version of search had too many options. This will help folks move through the search more effectively when on mobile


Additions

Added mobile classes to html
Added a rule for changing mobile facets

How to test this PR

  1. Go to the TDP Activity search, and scale down to mobile, then refresh the page, and notice if the facets work correctly.

Notes and todos

Need to move the JS rule to somewhere more appropriate

Checklist

  • PR has an informative and human-readable title
    • PR titles are used to generate the change log in releases; good ones make that easier to scan.
    • Consider prefixing, e.g., "Mega Menu: fix layout bug", or "Docs: Update Docker installation instructions".
  • Changes are limited to a single goal (no scope creep)
  • Code follows the standards laid out in the CFPB development guidelines
  • Future todos are captured in comments and/or tickets

Front-end testing

Browser testing

Visually tested in the following supported browsers:

  • Firefox
  • Chrome
  • Safari
  • Edge 18 (the last Edge prior to it switching to Chromium)
  • Internet Explorer 11 and 8 (via emulation in 11's dev tools)
  • Safari on iOS
  • Chrome on Android

Accessibility

  • Keyboard friendly (navigable with tab, space, enter, arrow keys, etc.)
  • Screen reader friendly
  • Does not introduce new errors or warnings in WAVE

Other

  • Is useable without CSS
  • Is useable without JS
  • Does not introduce new lint warnings
  • Flexible from small to large screens

@Scotchester Scotchester requested a review from dcmouyard May 4, 2021 18:29
@Scotchester
Copy link
Contributor

@karenkitchens FYI, I merged the main branch into this PR so that our dev preview deployment would hopefully complete successfully.

mrclay and others added 3 commits May 5, 2021 08:11
- Moves logic to own module
- Adds option to the "expandable" template
- Makes the search module a better team player by not
  initializing itself and instead allowing the index module to do this.
Formalize logic into feature of the template organism
@mrclay
Copy link
Contributor

mrclay commented May 5, 2021

@Scotchester FYI we've reworked this a bit and added a test.

* so some elements won't be expanded by default.
*/
function beforeExpandableTransitionInit() {
document.querySelectorAll( '.' + MOBILE_COLLAPSED_CLASS ).forEach( el => {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this line is breaking the functionality of the filters in IE 11. None of the faceting works as it used to, and I see this error in the console:

Object doesn't support property or method 'forEach'

Copy link
Contributor

Choose a reason for hiding this comment

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

(Affecting desktop widths, just to be clear.)

Copy link
Contributor

@mrclay mrclay May 13, 2021

Choose a reason for hiding this comment

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

@Scotchester I'll look into this right away. Is IE11 the baseline or (gulp) earlier versions?

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks, Steve! For JS-required functionality like this, we only need to support IE 11.

Copy link
Contributor

@Scotchester Scotchester left a comment

Choose a reason for hiding this comment

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

That did the trick! I'll get this merged and try to deploy it tomorrow.

@Scotchester Scotchester enabled auto-merge (squash) May 13, 2021 22:07
@Scotchester Scotchester merged commit 5b8aa83 into main May 13, 2021
@Scotchester Scotchester deleted the tdp-mobile-search-fixes branch May 13, 2021 22:10
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.

3 participants