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

Move state and territory filter #6490

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

patphongs
Copy link
Member

@patphongs patphongs commented Sep 25, 2024

Summary (required)

Per ticket specifications, re-arrange filters and update text accordingly.

Required reviewers

1 UX
1 front end developer

Impacted areas of the application

General components of the application that this PR will affect:

  • Individual contributions datatable filters
  • Receipt datatable filters

Screenshots

Receipts datatable

Screenshot 2024-09-25 at 5 15 24 PM Screenshot 2024-09-25 at 5 15 03 PM Screenshot 2024-09-25 at 5 14 33 PM Screenshot 2024-09-25 at 5 14 06 PM

Individual contributions datatable

Screenshot 2024-09-25 at 5 24 10 PM Screenshot 2024-09-25 at 5 23 55 PM Screenshot 2024-09-25 at 5 23 32 PM Screenshot 2024-09-25 at 5 41 49 PM

How to test

Copy link

codecov bot commented Sep 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.48%. Comparing base (738f129) to head (5a7c86a).
Report is 172 commits behind head on develop.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop    #6490   +/-   ##
========================================
  Coverage    80.48%   80.48%           
========================================
  Files          234      234           
  Lines         5093     5093           
========================================
  Hits          4099     4099           
  Misses         994      994           
Flag Coverage Δ
80.48% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@JonellaCulmer
Copy link
Contributor

@patphongs, I'm hoping we can handle each of the updated sections the same way. Here are (mostly) screenshots of the Time period accordion but the changes affect multiple sections. I'd like to change the margin-top (5) and padding-top (10) in two areas, as well as the line height in the paragraphs (1.5). This removes some of the extra space above the new paragraphs and tightens up the text a bit. There's also a type: accross. Please let me know if I should present this all differently to you.

Accordion padding top (10)

Screenshot 2024-10-01 at 5 24 52 PM

Label margin top (5)

Screenshot 2024-10-01 at 5 12 43 PM

Small typo

Screenshot 2024-10-01 at 5 13 01 PM

Line height (1.5)

Screenshot 2024-10-01 at 5 13 31 PM
Screenshot 2024-10-01 at 5 27 43 PM
Screenshot 2024-10-01 at 5 27 33 PM

{% import 'macros/filters/committee-types.jinja' as committee_type %}
{{ committee_type.field(committee_type='recipient_committee_type', organization_type='recipient_committee_org_type', designation='recipient_committee_designation') }}
</div>
<button type="button" class="js-accordion-trigger accordion__button">Contribution details</button>
<div class="accordion__content">
<label class="label--help u-margin--top">When searching by contribution details and across multiple time periods, choose one or more fields: <br /> ecipient name or ID, contributor name or ID, city, ZIP code, occupation, employer, or image number.</label>
Copy link
Contributor

Choose a reason for hiding this comment

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

'ecipient'? 😄

The issue ticket is inconsistent on 'occupation or employer, or' vs 'occupation or employer, or'

@@ -34,6 +34,7 @@ Filter receipts
<legend class="label u-margin--top">Source details</legend>
{{ typeahead.field(name = 'contributor_name', title = 'Name or ID', dataset='committees', allow_text=True, minor_label_text = True, helper_text = 'Limit 10 source names or IDs') }}
{{ text.field(name = 'contributor_city', title = 'City', minor_label_text = True, helper_text = 'Limit 10 cities') }}
{{ states.field('contributor_state', minor_label_text = True, helper_text = 'When searching by state or territory, and across multiple time periods, choose one or more fields: recipient name or ID, source name or ID, city, ZIP code, occupation, employer, or image number.', restricted_field = True) }}
Copy link
Contributor

Choose a reason for hiding this comment

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

The issue ticket is inconsistent on 'occupation or employer, or' vs 'occupation or employer, or'

@@ -42,22 +43,21 @@ Filter receipts
<div class="js-accordion accordion--neutral restricted-fields" data-content-prefix="filter" data-open-first="true">
<button type="button" class="js-accordion-trigger accordion__button">Time period</button>
<div class="accordion__content">
{{ years.cycles('two_year_transaction_period', 'Report time period', multi_time_period_label="When searching multiple time periods, choose one or more fields: recipient name or ID, source name or ID, city, ZIP code, occupation or employer, or image number.") }}
<label class="label--help u-margin--top">When searching accross multiple time periods, choose one or more fields: <br /> recipient name or ID, source name or ID, city, ZIP code, occupation, employer, or image number.</label>
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we remove a c from 'accross'?

The issue ticket is inconsistent on 'occupation or employer, or' vs 'occupation or employer, or'

<button type="button" class="js-accordion-trigger accordion__button">Recipient committee type</button>
<div class="accordion__content">
<label class="label--help u-margin--top">When searching by recipient committee type and across multiple time periods, choose one or more fields: <br /> recipient name or ID, source name or ID, city, ZIP code, occupation, employer, or image number.</label>
Copy link
Contributor

Choose a reason for hiding this comment

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

The issue ticket is inconsistent on 'occupation or employer, or' vs 'occupation or employer, or'

{% import 'macros/filters/committee-types.jinja' as committee_type %}
{{ committee_type.field(committee_type='recipient_committee_type', organization_type='recipient_committee_org_type', designation='recipient_committee_designation') }}
</div>
<button type="button" class="js-accordion-trigger accordion__button">Receipt details</button>
<div class="accordion__content">
<label class="label--help u-margin--top">When searching by receipt details and across multiple time periods, choose one or more fields: <br /> recipient name or ID, source name or ID, city, ZIP code, occupation, employer, or image number.</label>
Copy link
Contributor

Choose a reason for hiding this comment

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

The issue ticket is inconsistent on 'occupation or employer, or' vs 'occupation or employer, or'

<button type="button" class="js-accordion-trigger accordion__button">Recipient committee type</button>
<div class="accordion__content">
<label class="label--help u-margin--top">When searching by recipient committee type and across multiple time periods, choose one or more fields: <br /> recipient name or ID, contributor name or ID, city, ZIP code, occupation or employer, or image number.</label>
Copy link
Contributor

Choose a reason for hiding this comment

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

The issue ticket is inconsistent on 'occupation or employer, or' vs 'occupation or employer, or'

{{ text.field(name = 'contributor_city', title = 'City', minor_label_text = True, helper_text = 'Limit 10 cities') }}
{{ states.field('contributor_state', minor_label_text = True, helper_text = 'When searching by state or territory, and across multiple time periods, choose one or more fields: recipient name or ID, contributor name or ID, city, ZIP code, occupation, employer, or image number.', restricted_field = True) }}
Copy link
Contributor

Choose a reason for hiding this comment

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

The issue ticket is inconsistent on 'occupation or employer, or' vs 'occupation or employer, or'

{{ text.field(name = 'contributor_occupation', title = 'Occupation', minor_label_text = True, helper_text = 'Use the employer field to search by occupation in reports filed before 2003. Limit 10 occupations') }}
{{ text.field(name = 'contributor_employer', title = 'Employer', minor_label_text = True, helper_text = 'Limit 10 employers') }}
{{ text.field(name = 'image_number', title = 'Image number') }}
</div>
<div class="js-accordion accordion--neutral restricted-fields" data-content-prefix="filter" data-open-first="true">
<button type="button" class="js-accordion-trigger accordion__button">Time period</button>
<div class="accordion__content">
{{ years.cycles('two_year_transaction_period', 'Report time period', multi_time_period_label="When searching multiple time periods, choose one or more fields: recipient name or ID, contributor name, city, ZIP code, occupation or employer, or image number.") }}
<label class="label--help u-margin--top">When searching accross multiple time periods, choose one or more fields: <br /> recipient name or ID, contributor name or ID, city, ZIP code, occupation or employer, or image number.</label>
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we remove a c from 'accross'?

The issue ticket is inconsistent on 'occupation or employer, or' vs 'occupation or employer, or'

@@ -19,7 +19,7 @@ $(function() {
useExport: true,
rowCallback: modalRenderRow,
error400Message:
'<p>You&#39;re trying to search across multiple time periods. Filter by recipient name or ID, contributor details, or image number for results.</p>',
'<p><p>You&#39;re trying to search across multiple time periods. Filter by recipient name or ID, other contributor details, or image number.</p>',
Copy link
Contributor

Choose a reason for hiding this comment

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

<p><p>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

Move state or territory filter and other adjustments on receipts and individual contributions datatables
3 participants