Skip to content

Commit

Permalink
fix declarations using only the first value
Browse files Browse the repository at this point in the history
  • Loading branch information
Sharktheone committed Sep 19, 2024
1 parent 920d0a1 commit f8e6759
Show file tree
Hide file tree
Showing 5 changed files with 61 additions and 22 deletions.
19 changes: 14 additions & 5 deletions crates/gosub_css3/src/convert/ast_converter.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
use anyhow::anyhow;
use log::warn;

use gosub_shared::types::Result;

use crate::node::{Node as CssNode, NodeType};
use crate::stylesheet::{
AttributeSelector, Combinator, CssDeclaration, CssOrigin, CssRule, CssSelector,
CssSelectorPart, CssStylesheet, CssValue, MatcherType,
};
use anyhow::anyhow;
use gosub_shared::types::Result;
use log::warn;

/*
Expand Down Expand Up @@ -208,9 +210,15 @@ pub fn convert_ast_to_stylesheet(
continue;
}

let value = if css_values.len() == 1 {
css_values.pop().expect("unreachable")
} else {
CssValue::List(css_values)
};

rule.declarations.push(CssDeclaration {
property: property.clone(),
value: css_values.to_vec(),
value,
important: *important,
});
}
Expand All @@ -223,10 +231,11 @@ pub fn convert_ast_to_stylesheet(

#[cfg(test)]
mod tests {
use super::*;
use crate::parser_config::ParserConfig;
use crate::Css3;

use super::*;

#[test]
fn convert_font_family() {
let ast = Css3::parse(
Expand Down
4 changes: 2 additions & 2 deletions crates/gosub_css3/src/stylesheet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ pub struct CssDeclaration {
pub property: String,
// Raw values of the declaration. It is not calculated or converted in any way (ie: "red", "50px" etc)
// There can be multiple values (ie: "1px solid black" are split into 3 values)
pub value: Vec<CssValue>,
pub value: CssValue,
// ie: !important
pub important: bool,
}
Expand Down Expand Up @@ -472,7 +472,7 @@ mod test {
}],
declarations: vec![CssDeclaration {
property: "color".to_string(),
value: vec![CssValue::String("red".to_string())],
value: CssValue::String("red".to_string()),
important: false,
}],
};
Expand Down
2 changes: 1 addition & 1 deletion crates/gosub_html5/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ license = "MIT"
[dependencies]
gosub_shared = { path = "../gosub_shared", features = [] }
gosub_css3 = { path = "../gosub_css3", features = [] }
derive_more = { version = "1", features = ["from"] }
derive_more = { version = "1", features = ["from", "display"] }
phf = { version = "0.11.2", features = ["macros"] }
lazy_static = "1.5"
thiserror = "1.0.61"
Expand Down
23 changes: 21 additions & 2 deletions crates/gosub_styling/src/property_definitions.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
use std::collections::HashMap;
use std::sync::LazyLock;

use log::warn;

use gosub_css3::stylesheet::CssValue;

use crate::shorthands::{FixList, Shorthands};
use std::sync::LazyLock;

use crate::syntax::GroupCombinators::Juxtaposition;
use crate::syntax::{CssSyntax, SyntaxComponent};
use crate::syntax_matcher::CssSyntaxTree;
Expand Down Expand Up @@ -892,4 +891,24 @@ mod tests {
unit!(4.0, "px"),
]));
}

#[test]
fn test_font_var() {
let definitions = get_css_definitions();
let def = definitions
.find_property("font-variation-settings")
.unwrap();

assert_true!(def.matches(&[str!("normal")]));

assert_true!(def.matches(&[str!("wgth"), CssValue::Number(100.0)]));

assert_true!(def.matches(&[
str!("wgth"),
CssValue::Number(100.0),
CssValue::Comma,
str!("ital"),
CssValue::Number(100.0)
]));
}
}
35 changes: 23 additions & 12 deletions crates/gosub_styling/src/render_tree.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use std::collections::HashMap;
use std::fmt::{Debug, Formatter};
use std::slice;

use log::warn;

Expand Down Expand Up @@ -314,15 +315,22 @@ impl<L: Layouter> RenderTree<L> {

let value = resolve_functions(&declaration.value, node, &doc);

let match_value = if let CssValue::List(value) = &value {
&**value
} else {
slice::from_ref(&value)
};

// create property for the given values
let property_name = declaration.property.clone();

// Check if the declaration matches the definition and return the "expanded" order
let res = definition.matches_and_shorthands(&value, &mut fix_list);
let res = definition.matches_and_shorthands(match_value, &mut fix_list);
if !res {
warn!("Declaration does not match definition: {:?}", declaration);
continue;
}

// create property for the given values
let property_name = declaration.property.clone();
let decl = CssDeclaration {
property: property_name.to_string(),
value,
Expand Down Expand Up @@ -585,7 +593,7 @@ pub fn add_property_to_map(
//
let declaration = DeclarationProperty {
// @todo: this seems wrong. We only get the first values from the declared values
value: declaration.value.first().unwrap().clone(),
value: declaration.value.clone(),
origin: sheet.origin.clone(),
important: declaration.important,
location: sheet.location.clone(),
Expand Down Expand Up @@ -964,13 +972,11 @@ pub fn node_is_undernderable(node: &gosub_html5::node::Node) -> bool {
}

pub fn resolve_functions(
value: &[CssValue],
value: &CssValue,
node: &gosub_html5::node::Node,
doc: &Document,
) -> Vec<CssValue> {
let mut result = Vec::with_capacity(value.len()); //TODO: we could give it a &mut Vec and reuse the allocation

for val in value {
) -> CssValue {
fn resolve(val: &CssValue, node: &gosub_html5::node::Node, doc: &Document) -> CssValue {
match val {
CssValue::Function(func, values) => {
let resolved = match func.as_str() {
Expand All @@ -980,13 +986,18 @@ pub fn resolve_functions(
_ => vec![val.clone()],
};

result.extend(resolved);
CssValue::List(resolved)
}
_ => result.push(val.clone()),
_ => val.clone(),
}
}

result
if let CssValue::List(list) = value {
let resolved = list.iter().map(|val| resolve(val, node, doc)).collect();
CssValue::List(resolved)
} else {
resolve(value, node, doc)
}
}

// pub fn walk_render_tree(tree: &RenderTree, visitor: &mut Box<dyn TreeVisitor<RenderTreeNode>>) {
Expand Down

0 comments on commit f8e6759

Please sign in to comment.