topcoat-data / topcoat-public

TopCoat components, visualizations and themes
MIT License
1 stars 3 forks source link

fix: only add grid columns if collapsable has rows #249

Closed benbarnett closed 1 year ago

benbarnett commented 1 year ago

What this does

Following on from #248, there was another edge case where if the table initially loads data, and then some combination of applied filters means that the table returns no results -- then the template columns were required again.

I've tested this with:

Screenshots for the states that had the layout issue shown below;

Before (in main): Screenshot 2023-06-20 at 10 19 46

With this fix: Screenshot 2023-06-20 at 10 18 42

Notes for the reviewer

Instructions on how to run this locally, background context, what to review, best to review commit-by-commit?, questions…

Missed anything?

More information

Screenshots / GIFs

Visuals that may help the reviewer. Please add screenshots, including URL if applicable, for any front end change. GIFs are most welcome!

egcastro commented 1 year ago

Did you have a chance to test those cases on the PDF version of other reports? I know Insights it's not using the PDF feature but to see if these changes affect (for good or bad) the current PDF tables.

benbarnett commented 1 year ago

@egcastro I checked the Issues Detail and Vuln detail reports, exported both to PDF (standard & collapsable accordingly), and as far as I can tell everything aligns with prod (with data and without data)