Skip to content

Comments

Modernize CSS Styles for Text and Figures || chore: update code formatting and styles#10

Open
SH20RAJ wants to merge 2 commits intolowlighter:mainfrom
sh20raj-dump:main
Open

Modernize CSS Styles for Text and Figures || chore: update code formatting and styles#10
SH20RAJ wants to merge 2 commits intolowlighter:mainfrom
sh20raj-dump:main

Conversation

@SH20RAJ
Copy link

@SH20RAJ SH20RAJ commented May 25, 2024

This pull request modernizes the CSS styles for paragraphs, preformatted text, blockquotes, and figures to enhance readability and visual appeal. Key improvements include:

Paragraphs and Preformatted Text:

  • Improved font sizing and line height.
  • Added subtle shadows for preformatted text.
  • Enhanced padding and border-radius for a cleaner look.
  • Blockquotes:

Updated padding, border, and background for a modern appearance.
Added shadow and border-radius for better emphasis.
Figures:

  • Centered content using flexbox.
  • Applied consistent padding, background, and shadow for a cohesive design.
  • These changes aim to provide a more polished and contemporary user experience.

display: grid;
grid-template-columns: repeat(auto-fit, minmax(0, 1fr));
display: block;
width: 100%;
Copy link
Owner

Choose a reason for hiding this comment

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

The display: grid; grid-template-columns; cannot be removed actually, because it'll "break" the body size on small screens on many browsers as seen below:
image

For some reason even with overflow: auto this issue occurs, unless a better style exists to fix this issue, I think this should be kept (which include the removed block below)

/* Hover effects for better interactivity */
tbody tr:hover {
background: var(--bg-hover);
}
Copy link
Owner

Choose a reason for hiding this comment

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

The property --bg-hover doesn't exist so this has actually no effects

Also this was previously in matcha but I decided to remove it because it looks really weird when you start using rowspan attribute

}

table.center td, th {
table.center th, table.center td {
Copy link
Owner

Choose a reason for hiding this comment

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

IMHO th should still be centered even on non-centered tables, because they're table headers

}

td {
color: var(--text-color);
Copy link
Owner

Choose a reason for hiding this comment

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

This is variable doesn't exist, and this ruleset is probably not needed neither, cells are already using the --default color so this has no effect

background: var(--bg-subtle);
font-weight: bold;
font-weight: 600;
color: var(--primary-color);
Copy link
Owner

Choose a reason for hiding this comment

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

The --primary-color doesn't exist.
Also removing the borders on table headers make the table quite difficult to read (especially on colspan):
image

Reducing bolding, uppercasing with letter-spacing seems fine though.

line-height: 1.45;
font-size: 0.9rem;
line-height: 1.6;
box-shadow: inset 0 0 10px rgba(0, 0, 0, 0.1);
Copy link
Owner

Choose a reason for hiding this comment

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

This kind of value should inherit from --shadow property

border-left: .25rem solid var(--bd-muted);
margin: 0 0 1rem;
padding: 0.5rem 1.5rem;
border-left: 0.25rem solid var(--primary-color);
Copy link
Owner

Choose a reason for hiding this comment

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

The variable --primary-color doesn't exist
Also no need to add back 0 to numbers, as they don't really grant much readbility and actually increase a bit the resulting size of the file

border-left: 0.25rem solid var(--primary-color);
margin: 0 0 1.5rem;
color: var(--muted);
background: var(--bg-light);
Copy link
Owner

Choose a reason for hiding this comment

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

The variable --bg-light doens't exist, also I'm not convinced a blockquote should have a background color

margin-top: .25rem;
margin-top: 0.5rem;
text-decoration: none;
font-style: italic;
Copy link
Owner

Choose a reason for hiding this comment

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

To remove, this is already in italic

@SH20RAJ SH20RAJ requested a review from lowlighter May 29, 2024 03:41
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.

2 participants