Skip to content

Commit

Permalink
fix(css_formatter): fix CSS formatting for css values (#3153)
Browse files Browse the repository at this point in the history
  • Loading branch information
denbezrukov authored Jun 10, 2024
1 parent 746db0a commit b75d30e
Show file tree
Hide file tree
Showing 8 changed files with 137 additions and 123 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ our [guidelines for writing a good changelog entry](https://github.com/biomejs/b
#### Bug fixes
- Fix the bug where whitespace after the & character in CSS nesting was incorrectly trimmed, ensuring proper targeting of child classes [#3061](https://github.com/biomejs/biome/issues/3061). Contributed by @denbezrukov
- Fix [#3068](https://github.com/biomejs/biome/issues/3068) where the CSS formatter was inadvertently converting variable declarations and function calls to lowercase. Contributed by @denbezrukov
- Fix the formatting of CSS grid layout properties. Contributed by @denbezrukov

### JavaScript APIs

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ impl FormatNodeRule<CssBracketedValue> for FormatCssBracketedValue {
f,
[
l_brack_token.format(),
soft_block_indent(&items.format()),
items.format(),
r_brack_token.format()
]
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ impl FormatNodeRule<CssComposesProperty> for FormatCssComposesProperty {
colon_token,
value,
} = node.as_fields();

write!(
f,
[name.format(), colon_token.format(), space(), value.format()]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ impl FormatNodeRule<CssValueAtRuleGenericProperty> for FormatCssValueAtRuleGener
colon_token,
value,
} = node.as_fields();

write!(
f,
[name.format(), colon_token.format(), space(), value.format()]
Expand Down
74 changes: 71 additions & 3 deletions crates/biome_css_formatter/src/utils/component_value_list.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::comments::CssComments;
use biome_css_syntax::{CssGenericDelimiter, CssLanguage, CssSyntaxKind};
use biome_css_syntax::{CssGenericDelimiter, CssGenericProperty, CssLanguage, CssSyntaxKind};
use biome_formatter::FormatResult;
use biome_formatter::{write, CstFormatContext};

Expand All @@ -13,6 +13,21 @@ where
I: AstNode<Language = CssLanguage> + IntoFormat<CssFormatContext>,
{
let layout = get_value_list_layout(node, f.context().comments());

// Check if any of the elements in the list have a leading newline.
// We skip the first element because it is the first element in the list and should not be considered.
// div {
// grid-template-columns:
// 1fr 100px 3em;
// }
let has_newline = match layout {
ValueListLayout::PreserveInline => node
.iter()
.skip(1)
.any(|element| element.syntax().has_leading_newline()),
_ => false,
};

let values = format_with(|f: &mut Formatter<'_, CssFormatContext>| {
let mut fill = f.fill();

Expand All @@ -28,7 +43,17 @@ where
.map_or(false, |node| node.kind() == CssSyntaxKind::COMMA);

if !is_comma {
write!(f, [soft_line_break_or_space()])?
if matches!(layout, ValueListLayout::PreserveInline) {
let has_leading_newline = element.syntax().has_leading_newline();

if has_leading_newline {
write!(f, [hard_line_break()])?;
} else {
write!(f, [space()])?;
}
} else {
write!(f, [soft_line_break_or_space()])?
}
}

Ok(())
Expand All @@ -41,6 +66,17 @@ where
});

match layout {
ValueListLayout::PreserveInline => {
let content = format_once(|f| {
if has_newline {
// Add line break before the first element if we have more than two lines.
write!(f, [hard_line_break()])?;
}
write!(f, [values])
});

write!(f, [group(&indent(&content))])
}
ValueListLayout::Fill => {
write!(f, [group(&indent(&values))])
}
Expand Down Expand Up @@ -81,6 +117,25 @@ pub(crate) enum ValueListLayout {
/// ```
Fill,

/// Keeps elements on the same line if they're on the same line in the source file.
///
/// For example, this layout option is commonly used for CSS grid properties. It ensures that properties
/// remain on the same line in the formatted output if they were on the same line in the source file.
/// If a new line is encountered in the source file, a corresponding new line is added in the formatted
/// output at the beginning of the property.
///
/// # Example
///
/// ```css
/// grid-template-areas: 'header header' 'main sidebar' 'footer footer';
/// grid-template-columns:
/// [full-start] minmax(1.50em, 1fr)
/// [main-start] minmax(.40ch, 75ch)
/// [main-end] minmax(1em, 1.000fr)
/// [full-end];
/// ```
PreserveInline,

/// Prints every value on a single line if the whole list exceeds the line
/// width, or any of its elements gets printed in *expanded* mode.
/// ```css
Expand All @@ -105,8 +160,21 @@ where
N: AstNodeList<Language = CssLanguage, Node = I> + AstNode<Language = CssLanguage>,
I: AstNode<Language = CssLanguage> + IntoFormat<CssFormatContext>,
{
let is_grid_property = list
.parent::<CssGenericProperty>()
.and_then(|parent| parent.name().ok())
.and_then(|name| {
name.as_css_identifier()
.map(|name| name.text().to_lowercase())
})
.map_or(false, |name| {
name.starts_with("grid-template") || name == "grid"
});

// TODO: Check for comments, check for the types of elements in the list, etc.
if list.len() == 1 {
if is_grid_property {
ValueListLayout::PreserveInline
} else if list.len() == 1 {
ValueListLayout::SingleValue
} else {
ValueListLayout::Fill
Expand Down
7 changes: 2 additions & 5 deletions crates/biome_css_formatter/tests/quick_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,8 @@ mod language {
// use this test check if your snippet prints as you wish, without using a snapshot
fn quick_test() {
let src = r#"
.container {
&
.child {
color: blue;
}
div {
grid-template-columns: 1fr 100px 3em;
}
"#;
let parse = parse_css(src, CssParserOptions::default());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,11 @@ Quote style: Double Quotes
#grid {
display: grid;
grid-template-columns: [first nav-start] 150px [main-start] 1fr [last];
grid-template-rows: [first header-start] 50px [main-start] 1fr [footer-start]
50px [last];
grid-template-rows: [first header-start] 50px [main-start] 1fr [footer-start] 50px [last];
}
```

# Lines exceeding max width of 80 characters
```
9: grid-template-rows: [first header-start] 50px [main-start] 1fr [footer-start] 50px [last];
```
166 changes: 54 additions & 112 deletions crates/biome_css_formatter/tests/specs/prettier/css/grid/grid.css.snap
Original file line number Diff line number Diff line change
Expand Up @@ -128,135 +128,61 @@ div {
```diff
--- Prettier
+++ Biome
@@ -1,40 +1,26 @@
/* quotes */
div {
- grid-template-areas:
- "header header"
- "main sidebar"
- "footer footer";
+ grid-template-areas: "header header" "main sidebar" "footer footer";
}

@@ -9,9 +9,9 @@
/* numbers */
div {
- grid-template-columns:
grid-template-columns:
- [full-start] minmax(1.5em, 1fr)
- [main-start] minmax(0.4ch, 75ch)
- [main-end] minmax(1em, 1fr)
- [full-end];
+ grid-template-columns: [full-start] minmax(1.50em, 1fr) [main-start]
+ minmax(.40ch, 75ch) [main-end] minmax(1em, 1.000fr) [full-end];
}

/* casing */
div {
- grid:
- [top] 1fr
- [middle] 1fr
- bottom;
+ grid: [top] 1fr [middle] 1fr bottom;

- grid-template:
- "a a a" 200px
- "b b b" 200px
- / 200px 200px auto;
+ grid-template: "a a a" 200px "b b b" 200px / 200px 200px auto;
+ [full-start] minmax(1.50em, 1fr)
+ [main-start] minmax(.40ch, 75ch)
+ [main-end] minmax(1em, 1.000fr)
[full-end];
}

/* break before first line if there are any breaks */
div {
grid-template-columns: 1fr 100px 3em;
- grid:
- [wide-start] "header header header" 200px [wide-end]
- "footer footer footer" 25px
- / auto 50px auto;
+ grid: [wide-start] "header header header" 200px [wide-end]
+ "footer footer footer" 25px / auto 50px auto;
}

/**
@@ -47,42 +33,26 @@
grid-template-columns: 1fr 100px 3em;
grid-template-rows: 1fr 100px 3em;
/* template rows/columns with named grid lines */
- grid-template-columns:
- [wide-start] 1fr
- [main-start] 500px
- [main-end] 1fr
+ grid-template-columns: [wide-start] 1fr [main-start] 500px [main-end] 1fr
[wide-end];
- grid-template-rows:
- [top] 1fr
- [middle] 1fr
- [bottom];
+ grid-template-rows: [top] 1fr [middle] 1fr [bottom];
/* template rows/columns with functions */
grid-template-columns: minmax(1em, 1fr) minmax(1em, 80ch) minmax(1em, 1fr);
/* getting really busy with named lines + functions */
- grid-template-columns:
- [full-start] minmax(1em, 1fr)
- [main-start] minmax(1em, 80ch)
- [main-end] minmax(1em, 1fr)
- [full-end];
+ grid-template-columns: [full-start] minmax(1em, 1fr) [main-start]
+ minmax(1em, 80ch) [main-end] minmax(1em, 1fr) [full-end];

- grid-template-areas:
- "header header header"
- "main main sidebar"
- "main main sidebar2"
- "footer footer footer";
+ grid-template-areas: "header header header" "main main sidebar"
+ "main main sidebar2" "footer footer footer";

/* Shorthand for grid-template-rows, grid-template-columns, and grid-template
areas. In one. This can get really crazy. */
- grid-template:
- [row1-start] "header header header" 25px [row1-end]
- [row2-start] "footer footer footer" 25px [row2-end]
- / auto 50px auto;
+ grid-template: [row1-start] "header header header" 25px [row1-end]
+ [row2-start] "footer footer footer" 25px [row2-end] / auto 50px auto;

/* The. Worst. This one is shorthand for like everything here smashed into one. But rarely will you actually specify EVERYTHING. */
- grid:
- [row1-start] "header header header" 25px [row1-end]
- [row2-start] "footer footer footer" 25px [row2-end]
- / auto 50px auto;
+ grid: [row1-start] "header header header" 25px [row1-end] [row2-start]
+ "footer footer footer" 25px [row2-end] / auto 50px auto;
/* simpler use case: */
grid: 200px auto / 1fr auto 1fr;

```

# Output

```css
/* quotes */
div {
grid-template-areas: "header header" "main sidebar" "footer footer";
grid-template-areas:
"header header"
"main sidebar"
"footer footer";
}

/* numbers */
div {
grid-template-columns: [full-start] minmax(1.50em, 1fr) [main-start]
minmax(.40ch, 75ch) [main-end] minmax(1em, 1.000fr) [full-end];
grid-template-columns:
[full-start] minmax(1.50em, 1fr)
[main-start] minmax(.40ch, 75ch)
[main-end] minmax(1em, 1.000fr)
[full-end];
}

/* casing */
div {
grid: [top] 1fr [middle] 1fr bottom;
grid:
[top] 1fr
[middle] 1fr
bottom;

grid-template: "a a a" 200px "b b b" 200px / 200px 200px auto;
grid-template:
"a a a" 200px
"b b b" 200px
/ 200px 200px auto;
}

/* break before first line if there are any breaks */
div {
grid-template-columns: 1fr 100px 3em;
grid: [wide-start] "header header header" 200px [wide-end]
"footer footer footer" 25px / auto 50px auto;
grid:
[wide-start] "header header header" 200px [wide-end]
"footer footer footer" 25px
/ auto 50px auto;
}

/**
Expand All @@ -269,26 +195,42 @@ div {
grid-template-columns: 1fr 100px 3em;
grid-template-rows: 1fr 100px 3em;
/* template rows/columns with named grid lines */
grid-template-columns: [wide-start] 1fr [main-start] 500px [main-end] 1fr
grid-template-columns:
[wide-start] 1fr
[main-start] 500px
[main-end] 1fr
[wide-end];
grid-template-rows: [top] 1fr [middle] 1fr [bottom];
grid-template-rows:
[top] 1fr
[middle] 1fr
[bottom];
/* template rows/columns with functions */
grid-template-columns: minmax(1em, 1fr) minmax(1em, 80ch) minmax(1em, 1fr);
/* getting really busy with named lines + functions */
grid-template-columns: [full-start] minmax(1em, 1fr) [main-start]
minmax(1em, 80ch) [main-end] minmax(1em, 1fr) [full-end];
grid-template-columns:
[full-start] minmax(1em, 1fr)
[main-start] minmax(1em, 80ch)
[main-end] minmax(1em, 1fr)
[full-end];

grid-template-areas: "header header header" "main main sidebar"
"main main sidebar2" "footer footer footer";
grid-template-areas:
"header header header"
"main main sidebar"
"main main sidebar2"
"footer footer footer";

/* Shorthand for grid-template-rows, grid-template-columns, and grid-template
areas. In one. This can get really crazy. */
grid-template: [row1-start] "header header header" 25px [row1-end]
[row2-start] "footer footer footer" 25px [row2-end] / auto 50px auto;
grid-template:
[row1-start] "header header header" 25px [row1-end]
[row2-start] "footer footer footer" 25px [row2-end]
/ auto 50px auto;

/* The. Worst. This one is shorthand for like everything here smashed into one. But rarely will you actually specify EVERYTHING. */
grid: [row1-start] "header header header" 25px [row1-end] [row2-start]
"footer footer footer" 25px [row2-end] / auto 50px auto;
grid:
[row1-start] "header header header" 25px [row1-end]
[row2-start] "footer footer footer" 25px [row2-end]
/ auto 50px auto;
/* simpler use case: */
grid: 200px auto / 1fr auto 1fr;

Expand Down Expand Up @@ -322,5 +264,5 @@ div {

# Lines exceeding max width of 80 characters
```
53: /* The. Worst. This one is shorthand for like everything here smashed into one. But rarely will you actually specify EVERYTHING. */
81: /* The. Worst. This one is shorthand for like everything here smashed into one. But rarely will you actually specify EVERYTHING. */
```

0 comments on commit b75d30e

Please sign in to comment.