Skip to content

Fix orphaned table footer labels and restore input-label lint - #1348

Merged
jaredgalanis merged 2 commits into
eclipse-pass:mainfrom
uvalib:1259-fix-passtable-form-labels
Aug 19, 2026
Merged

Fix orphaned table footer labels and restore input-label lint#1348
jaredgalanis merged 2 commits into
eclipse-pass:mainfrom
uvalib:1259-fix-passtable-form-labels

Conversation

@dougchestnut

@dougchestnut dougchestnut commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Summary

Corrects form-label markup in PassTable (Grants, Grant Details, Submissions) and turns require-input-label back on for this template.

  • Removed labels that are not labels. “Show N - M of T” is result text, so it is a <span>. When the Rows or Page select is not rendered, those captions are <span>s as well.
  • Associated labels with their controls. Search, Rows, and Page use <label for="…"> and matching ids on the input/select when those controls exist.
  • Restored linting. Dropped require-input-label from the template-lint disable so ember-template-lint checks this file again. no-invalid-interactive stays disabled (unrelated).

Fixes eclipse-pass/main#1259

How to test

  1. Run pass-ui locally (Mirage is enough).
  2. Grants (filter + both selects): “Search:”, “Rows:”, and “Page:” are labels whose for matches the control id. “Show …” is a <span>.
  3. Submissions or Grant Details (no Rows/Page select): “Show …”, “Rows:”, and “Page:” are <span>s. No <label> without a control.
  4. Confirm search, clear filter, page size, and page navigation still work.
  5. Run template lint — require-input-label should pass for pass-table.

Associate Search, Rows, and Page with their controls via for/id.
Use spans for result text and for Rows/Page when no select is rendered.
Re-enable require-input-label on the pass-table template.
@markpatton

Copy link
Copy Markdown
Contributor

Works for me locally. I will ask someone with a better ui knowledge to do a review.

@jaredgalanis jaredgalanis left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for wiring up the label associations! One concern with the approach: the three id values (pass-table-filter-input, pass-table-page-size-select, pass-table-page-select) are hardcoded strings in a reusable component, so any page that renders two PassTables at once will emit duplicate ids.

Since label[for] resolves document-wide in DOM order, every label in the second table would target the first table's controls — clicking "Search:" on table 2 focuses table 1's filter input, and the same goes for the "Rows:" and "Page:" selects. We're safe today only by coincidence: grants/index.gts invokes PassTable twice, but inside mutually exclusive {{#if isAdmin}}/{{else if isSubmitter}} branches, so only one instance is ever in the DOM at a time. Nothing guards this — no-duplicate-id only lints within a single template.

Could we use Ember's built-in uniqueId helper (from @ember/helper, available on our ember-source) instead? Something like:

{{#let (uniqueId) as |filterId|}}
  <label class='input-group-text' for={{filterId}}>Search:</label>
  <input id={{filterId}} type='text' class='filterString form-control' ... />
{{/let}}

Same pattern for the two selects. That keeps the label→control association this PR is adding while making each table instance's ids unique.

Hardcoded ids would collide if two tables rendered at once; label[for]
resolves document-wide. Ember uniqueId keeps each instance distinct.
@sonarqubecloud

Copy link
Copy Markdown

@dougchestnut

Copy link
Copy Markdown
Contributor Author

Good catch! I switched Search, Rows, and Page to Ember uniqueId so each PassTable instance gets its own ids. label[for] should associate correctly now when more than one table is in the DOM.

@jaredgalanis jaredgalanis left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good! 👍

@jaredgalanis
jaredgalanis merged commit d961782 into eclipse-pass:main Aug 19, 2026
6 checks passed
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.

Correct PassTable form labels and restore require-input-label linting

3 participants