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

Remove legacy reporting dates JS #6512

Merged

Conversation

johnnyporkchops
Copy link
Contributor

@johnnyporkchops johnnyporkchops commented Oct 11, 2024

Summary (required)

NOTE: Please merge this PR before merging #6597

Required reviewers

one frontend and optionally one content (@djgarr)

Impacted areas of the application

Reporting dates tables from 2020 to present

deleted: fec/static/js/pages/election-reporting-dates-tables.js
modified: fec/static/js/pages/reporting-dates-tables.js
modified: home/templates/home/full_width_page.html
modified: home/templates/home/reporting_dates_table.html

Related issues/PRs

Issues:
#5153

PRs:
New:
#5786
#5798
Legacy:
#3227

How to test

@patphongs patphongs changed the title Remove legacy reporting dates JS [DO NOT MERGE - WAITING ON TESTING] Remove legacy reporting dates JS Nov 26, 2024
@johnnyporkchops johnnyporkchops changed the title [DO NOT MERGE - WAITING ON TESTING] Remove legacy reporting dates JS Remove legacy reporting dates JS Jan 4, 2025
@johnnyporkchops johnnyporkchops merged commit 3f7391d into develop Jan 7, 2025
3 checks passed
@@ -11,7 +11,7 @@ const states_dropdown_template = `
<option value="CO">Colorado</option>
<option value="CT">Connecticut</option>
<option value="DE">Delaware</option>
<option value="DC">District Of Columbia</option>
<option value="DC">District of Columbia</option>
Copy link
Contributor

Choose a reason for hiding this comment

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

❤️

@@ -164,49 +168,26 @@ ReportingDates.prototype.buildStaticElements = function() {

table_parent.insertBefore(dropdown_wrapper, this.dates_table);

//Create static footnote/header note list
//Create header note list for modal dialogue
let hdr_str = '';
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 run the formatter on this file to fix the indentation spacing and spaces between functions' ){

ReportingDates.prototype.handleStateChange = function() {
const state = this.states.value.toLowerCase();

//TODO: Should this be `this.dates_table.querySelectorAll` ?
Copy link
Contributor

Choose a reason for hiding this comment

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

If it works, I always prefer more specific selectors. In this case, 'tr' would grab all rows on every table of the page, good or bad. 'tr.footnote_row' would likewise grab every tr with that class, in every table on the page

const date_sups = document.querySelectorAll('td sup');

if (typeof footnotes_array == 'object') {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to handle the else here or just skip them it's not an object? (Hasn't typeof been tricky lately, identifying Arrays as objects, too?)

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

Successfully merging this pull request may close these issues.

Tech-debt: Remove reporting-dates-tables.js by converting legacy tables to use new template and JS
2 participants