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

Fix: styles for tables in SQL lessons #10

Merged
merged 2 commits into from
Feb 25, 2015
Merged

Fix: styles for tables in SQL lessons #10

merged 2 commits into from
Feb 25, 2015

Conversation

pbanaszkiewicz
Copy link
Contributor

This PR addresses #9.

Previously

  • table cells had no padding, which caused a visual collapse (cell content was too close to cell neighbors)
  • table header rows had dark blue background and black letters
  • tables had no space between them and following paragraphs.

BEFORE:
screenshot from 2015-02-13 20 46 42

Changes

  • added 5px padding to cell contents
  • changed header row background to gray-ish (I considered switching text color to white, but it didn't look good)
  • 15px margin on the bottom of the tables (similarly to bottom margin used by paragraph style).

AFTER:
screenshot from 2015-02-13 20 51 07

Previously: table cells had no padding, which caused a visual collapse
(cell content was too close to cell neighbors). Table header rows had
dark blue background and black letters.  Tables had no space between
them and following paragraphs.

Changes:
* added 5px padding to cell contents
* changed header row background to gray-ish (I considered switching text
  color to white, but it didn't look good)
* 15px margin on the bottom of the tables (similarly to bottom margin
  used by paragraph style).
@rgaiacs
Copy link
Contributor

rgaiacs commented Feb 13, 2015

@pbanaszkiewicz What about use the same style from the table at our workshop template, see https://swcarpentry.github.io/workshop-template/? You should keep the change for the table head.

@pbanaszkiewicz
Copy link
Contributor Author

@r-gaia-cs thanks for suggestion. I was considering a switch to Bootstrap styles, but I don't entirely like them:

  1. they have huge horizontal padding
  2. they tinker with :last-child and :first-child a lot

If you have specific styles in mind that we can borrow (big padding, no background for <thead>, striped rows) please let me know and I'll implement it.

@rgaiacs
Copy link
Contributor

rgaiacs commented Feb 14, 2015

@pbanaszkiewicz I'm OK with the currently horizontal padding and agree to avoid playing a lot with :last-child and :first-child. What I'm suggesting is that we use

tbody > tr:nth-of-type(2n+1) {
    background-color: #F9F9F9;
}

and

tbody > tr > td {
    border-top: 1px solid #DDD;
}

This samples need improvements to be more robust.

Now the style is matching closely `.table-bordered` from Bootstrap.
I decided to leave the header row background blank because it was too
close in color to `#F9F9F9` - our new rule for even rows in the table.

I also increased horizontal padding by 5px on both sides, because now
that we have vertical borders it was too cramped.

Additionaly I introduced borders for every cell in the table.
@pbanaszkiewicz
Copy link
Contributor Author

Hi @r-gaia-cs,

this is what I have now:
screenshot from 2015-02-14 11 45 21

It's matching closely .table-bordered from Bootstrap. I decided to leave the header row background blank because it was too close in color to #F9F9F9 - our rule for even rows in the table.

I also increased horizontal padding by 5px on both sides, because now that we have vertical borders it was too cramped.

@pbanaszkiewicz pbanaszkiewicz mentioned this pull request Feb 14, 2015
rgaiacs pushed a commit that referenced this pull request Feb 25, 2015
Fix: styles for tables in SQL lessons
@rgaiacs rgaiacs merged commit 843999c into carpentries:master Feb 25, 2015
rgaiacs pushed a commit to rgaiacs/swc-styles that referenced this pull request Mar 14, 2017
…-slideshow

Removing motivational slideshow
fmichonneau added a commit that referenced this pull request Oct 20, 2020
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.

None yet

2 participants