-
Notifications
You must be signed in to change notification settings - Fork 1k
USWDS - Table: Simplify scrollable table example for site #5783
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same table data from usa-table.json
.
This allows us to:
- Reliably compile HTML templates for website.
- Dynamically create table using default
usa-table.twig
. - Get rid of the static
usa-table--scrollable.twig
template.
Note
You can re-use usa-table.json
in StorybookJS, but causes regressions on HTML templates. On compile (npm run build:html
) it will result in empty scrollable table variants.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for that note! It is helpful to know why it was set up this way.
I would be interested to investigate why and see if there is a way in the future to re-use some of this table JSON since we repeat so much, but that is out of scope for this issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not needed anymore since we're taking advantage of the dynamically generated table in usa-table.twig
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Caution
Inspect all compiled HTML templates for table to ensure there aren't regressions on site.
Should generate markup in html-templates
directory for website.
Simple scrollable table
Notice, multi-column headers are gone.
Compiled HTML - usa-table--scrollable.html
<!-- html-templates/usa-table--scrollable.html -->
<div class="usa-table-container--scrollable" tabindex="0">
<table class="usa-table">
<caption>
Scrollable table
</caption>
<thead>
<tr>
<th scope="col">Document title</th>
<th scope="col">Description</th>
<th scope="col">Year</th>
</tr>
</thead>
<tbody>
<tr>
<th scope="row">Declaration of Independence</th>
<td>
Statement adopted by the Continental Congress declaring independence
from the British Empire.
</td>
<td>1776</td>
</tr>
<tr>
<th scope="row">Bill of Rights</th>
<td>
The first ten amendments of the U.S. Constitution guaranteeing rights
and freedoms.
</td>
<td>1791</td>
</tr>
<tr>
<th scope="row">Declaration of Sentiments</th>
<td>
A document written during the Seneca Falls Convention outlining the
rights that American women should be entitled to as citizens.
</td>
<td>1848</td>
</tr>
<tr>
<th scope="row">Emancipation Proclamation</th>
<td>
An executive order granting freedom to slaves in designated southern
states.
</td>
<td>1863</td>
</tr>
</tbody>
</table>
</div>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me! I've performed the following tests:
- Check that the scrollable table markup has
usa-table-container--scrollable
wrapper (Checked in both storybook andhtml-templates
) - Confirm no visual regressions in any table variant
- Perform a code diff on the
html-template
markup for all table variants to confirm no changes to the markup - Confirm table styles look good when this branch is installed on uswds-site
- Confirm there are no conflicts with sticky table header PR (#5420)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for that note! It is helpful to know why it was set up this way.
I would be interested to investigate why and see if there is a way in the future to re-use some of this table JSON since we repeat so much, but that is out of scope for this issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks for flagging the html build error when you include usa-table.json
. I was hoping to be able to do the same!
Testing checklist
- Scrollable tables have the correct wrappers
- Zero regressions in other table markup
-
html-templates
have zero regressions - Installing on site properly updates component previews and component code
Summary
Address a11y issue in table demos on website. For now, a simple table will be used and an example of a complex table will come in the future.
Breaking change
This is not a breaking change.
Related issue
Closes uswds/uswds-site#2261.
Related pull requests
Changelog entry: uswds/uswds-site#2501.
Preview link
Preview link:
Problem statement
Currently, there's an A11Y issue with table example in scrollable variant on the website. The year header for 2019 doesn't have associated data cells underneath.
Solution
Replaced complex table with simpler table used in default variant.
Major changes
Extended default
usa-table.twig
template in favor of separate hardcodedusa-table--scrollable.twig
template [84256e3].Scrollable template changes
Testing and review
usa-table-container--scrollable
wrapper.npm run build:html
compiles correctly for scrollable and all variants.Checklist
usa-table-container--scrollable
.html-templates/
compile correct markup.