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.

Sign up for GitHub

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

minor tweaks to the style engine #43111

Merged
merged 1 commit into from
Aug 10, 2022
Merged

minor tweaks to the style engine #43111

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
16 changes: 5 additions & 11 deletions packages/style-engine/class-wp-style-engine-css-declarations.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,6 @@ class WP_Style_Engine_CSS_Declarations {
* @param array $declarations An array of declarations (property => value pairs).
*/
public function __construct( $declarations = array() ) {
if ( empty( $declarations ) ) {
return;
}
Comment on lines -37 to -39
Copy link
Member Author

Choose a reason for hiding this comment

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

If empty, then add_declarations won't add anything. This check doesn't serve any purpose.

$this->add_declarations( $declarations );
}

Expand Down Expand Up @@ -94,12 +91,12 @@ public function add_declarations( $declarations ) {
/**
* Remove multiple declarations.
*
* @param array $declarations An array of properties.
* @param array $properties An array of properties.
*
* @return void
*/
public function remove_declarations( $declarations = array() ) {
foreach ( $declarations as $property ) {
public function remove_declarations( $properties = array() ) {
foreach ( $properties as $property ) {
Comment on lines -97 to +99
Copy link
Member Author

Choose a reason for hiding this comment

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

Simply renamed $declarations to $properties since it's more accurate.

$this->remove_declaration( $property );
}
}
Expand All @@ -123,10 +120,7 @@ public function get_declarations() {
* @return string The filtered declaration as a single string.
*/
protected static function filter_declaration( $property, $value, $spacer = '' ) {
if ( isset( $property ) && isset( $value ) ) {
return safecss_filter_attr( "{$property}:{$spacer}{$value}" );
}
return '';
return safecss_filter_attr( "{$property}:{$spacer}{$value}" );
Comment on lines -126 to +123
Copy link
Member Author

Choose a reason for hiding this comment

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

$property and $value will always be set, they are mandatory params of the method.

Copy link
Member

Choose a reason for hiding this comment

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

Are there checks somewhere else?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes! We check the property and value when adding them to the object, so we already know they're there 😉

}

/**
Expand All @@ -143,9 +137,9 @@ public function get_declarations_string( $should_prettify = false, $indent_count
$indent = $should_prettify ? str_repeat( "\t", $indent_count ) : '';
$suffix = $should_prettify ? ' ' : '';
$suffix = $should_prettify && $indent_count > 0 ? "\n" : $suffix;
$spacer = $should_prettify ? ' ' : '';

foreach ( $declarations_array as $property => $value ) {
$spacer = $should_prettify ? ' ' : '';
Comment on lines +140 to -148
Copy link
Member Author

Choose a reason for hiding this comment

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

Moved this outside the loop. The $spacer's value doesn't change on each iteration of the loop so it can be defined outside it.

$filtered_declaration = static::filter_declaration( $property, $value, $spacer );
if ( $filtered_declaration ) {
$declarations_output .= "{$indent}{$filtered_declaration};$suffix";
Expand Down
6 changes: 1 addition & 5 deletions packages/style-engine/class-wp-style-engine-css-rule.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,14 +51,10 @@ public function __construct( $selector = '', $declarations = array() ) {
*
* @param string $selector The CSS selector.
*
* @return WP_Style_Engine_CSS_Rule|void Returns the object to allow chaining of methods.
* @return WP_Style_Engine_CSS_Rule Returns the object to allow chaining of methods.
*/
public function set_selector( $selector ) {
if ( empty( $selector ) ) {
return;
}
$this->selector = $selector;

Comment on lines -54 to -61
Copy link
Member Author

Choose a reason for hiding this comment

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

Always return the object to avoid fatalities if chaining methods.

return $this;
}

Expand Down
2 changes: 0 additions & 2 deletions packages/style-engine/class-wp-style-engine-css-rules-store.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ class WP_Style_Engine_CSS_Rules_Store {
*/
protected static $stores = array();


/**
* The store name.
*
Expand Down Expand Up @@ -114,7 +113,6 @@ public function get_all_rules() {
* @return WP_Style_Engine_CSS_Rule|null Returns a WP_Style_Engine_CSS_Rule object, or null if the selector is empty.
*/
public function add_rule( $selector ) {

$selector = trim( $selector );

// Bail early if there is no selector.
Expand Down
2 changes: 1 addition & 1 deletion packages/style-engine/class-wp-style-engine-processor.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ public function add_rules( $css_rules ) {
public function get_css( $options = array() ) {
$defaults = array(
'optimize' => true,
'prettify' => false,
'prettify' => defined( 'SCRIPT_DEBUG' ) && SCRIPT_DEBUG,
Copy link
Member Author

Choose a reason for hiding this comment

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

Use SCRIPT_DEBUG to determine the value of prettify by default.

);
$options = wp_parse_args( $options, $defaults );

Expand Down
8 changes: 2 additions & 6 deletions packages/style-engine/class-wp-style-engine.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -264,11 +264,7 @@ protected static function is_valid_style_value( $style_value ) {
return true;
}

if ( empty( $style_value ) ) {
return false;
}

return true;
return ! empty( $style_value );
Copy link
Member Author

Choose a reason for hiding this comment

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

Just simplifying code a bit

}

/**
Expand Down Expand Up @@ -512,7 +508,7 @@ public static function compile_css( $css_declarations, $css_selector ) {
public static function compile_stylesheet_from_css_rules( $css_rules ) {
$processor = new WP_Style_Engine_Processor();
$processor->add_rules( $css_rules );
return $processor->get_css( array( 'prettify' => defined( 'SCRIPT_DEBUG' ) && SCRIPT_DEBUG ) );
return $processor->get_css();
Comment on lines -515 to +511
Copy link
Member Author

Choose a reason for hiding this comment

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

Moved this check to the get_css() method itself

}
}

Expand Down