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

Add emptyItemsMessage, reduce CSS specificity for Table (table pattern) #193

Merged
merged 2 commits into from
Sep 15, 2021

Conversation

lyzadanger
Copy link
Contributor

This PR makes two updates to the Table component and its underlying table design pattern:

  • It reduces the specificity of the CSS rules in the table pattern to meet our general specificity guidelines and to make sure these styles are override-able in applications. Fixes Reduce specificity of table pattern styling #180
  • It adds a prop, emptyItemsMessage to the Table component. This arose out of the struggle to render such a message in the LMS FileList and having a heck of a time of it. Woulda been far easier if the Table could handle appropriately rendering and positioning the message. Fixes Add support for a "no items message" to Table #191

These changes will allow several small, helpful updates in the LMS application.

Make sure we're meeting our general specificity contract for styles that
should be override-able by applications. A style that applies to table
headers was nested too deep, making its resulting rule too high in
specificity, and making it thus hard to override in other applications.

Do a bit of rearranging and leveling out of other rules.

Fixes #180
@lyzadanger lyzadanger requested a review from esanzgar September 14, 2021 19:28
Copy link
Contributor

@esanzgar esanzgar left a comment

Choose a reason for hiding this comment

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

👍🏼

src/components/Table.js Outdated Show resolved Hide resolved
styles/mixins/patterns/_tables.scss Show resolved Hide resolved
Allow the ability to render a message if `items` is empty (and the
`Table` is not in a loading state). This obviates the need for parent
components to wrangle the message quite as much and simplifies and
standardizes styling/position of the message.

Fixes #191
@codecov
Copy link

codecov bot commented Sep 15, 2021

Codecov Report

Merging #193 (a50b437) into main (32e2999) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##              main      #193   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           14        14           
  Lines          225       227    +2     
  Branches        76        78    +2     
=========================================
+ Hits           225       227    +2     
Impacted Files Coverage Δ
src/components/Table.js 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 32e2999...a50b437. Read the comment docs.

@lyzadanger lyzadanger merged commit bb837d2 into main Sep 15, 2021
@lyzadanger lyzadanger deleted the table-updates branch September 15, 2021 12:57
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.

Add support for a "no items message" to Table Reduce specificity of table pattern styling
2 participants