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 declarations using only the first value #607

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading