Skip to content
This repository has been archived by the owner on Jul 10, 2020. It is now read-only.

Fix: Stacking tables on small screens #470

Merged
merged 2 commits into from
Apr 21, 2017
Merged

Conversation

mistergone
Copy link
Member

@mistergone mistergone commented Apr 19, 2017

This PR adds a small fix for stacking tables on small screens. Byspecifying width: 100%, we override a fixed width for the column, forcing the columns to use the full width available when stacked.

Additions

  • LESS in the .o-table__stack-on-small that adds width: 100% to th, td

Review

Screenshots

Before

(Second and third columns are width: 20%)

screen shot 2017-04-11 at 2 03 21 pm

After

(Second and third columns are width: 20%)

screen shot 2017-04-18 at 2 25 22 pm

Notes

  • I have a slight concern that this rule might not be specific enough to override other CSS rules. However, it's internally consistent in capital-framework. That is, if you put a class like u-w25pct on a cell, it will be overridden by this rule on small screens.

Checklist

  • Changes are limited to a single goal (no scope creep)
  • Code can be automatically merged (no conflicts)
  • Code follows the standards laid out in the front end playbook
  • Passes all existing automated tests
  • New functions include new tests
  • New functions are documented (with a description, list of inputs, and expected output)
  • Placeholder code is flagged
  • Visually tested in supported browsers and devices
  • Project documentation has been updated (including the "Unreleased" section of the CHANGELOG)

specifying width: 100%, we override a fixed width for the column, forcing
the columns to use the full width available when stacked.
Copy link
Contributor

@jimmynotjim jimmynotjim left a comment

Choose a reason for hiding this comment

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

Thanks for getting this fixed, looks good. Before we merge it could you add a a patched changelog entry? It'll look like

**cf-tables:** [PATCH] Description

@mistergone
Copy link
Member Author

CHANGELOG updated!

Copy link
Contributor

@Scotchester Scotchester left a comment

Choose a reason for hiding this comment

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

Looks good; thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants