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

Allow CSS functions to be used when expanding dimensions shorthand #126

Merged
merged 2 commits into from Jul 27, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion CHANGELOG.md
Expand Up @@ -2,7 +2,7 @@

### Unreleased

* Put diff here
* Allow CSS functions to be used in CssParser::RuleSet#expand_dimensions_shorthand! [#126](https://github.com/premailer/css_parser/pull/126)

### Version 1.9.0

Expand Down
16 changes: 16 additions & 0 deletions lib/css_parser/regexps.rb
Expand Up @@ -58,6 +58,22 @@ def self.regex_possible_values(*values)
RE_BORDER_STYLE = /(\s*^)?(none|hidden|dotted|dashed|solid|double|dot-dash|dot-dot-dash|wave|groove|ridge|inset|outset)(\s*$)?/imx.freeze
RE_BORDER_UNITS = Regexp.union(BOX_MODEL_UNITS_RX, /(thin|medium|thick)/i)

# Functions like calc, var, clamp, etc.
RE_FUNCTIONS = /
(
[a-z0-9-]+ # function name
)
(?>
\( # opening parenthesis
(?:
([^()]+)
| # recursion via subexpression
\g<0>
)*
\) # closing parenthesis
)
/imx.freeze

# Patterns for specificity calculations
NON_ID_ATTRIBUTES_AND_PSEUDO_CLASSES_RX_NC = /
(?:\.\w+) # classes
Expand Down
8 changes: 7 additions & 1 deletion lib/css_parser/rule_set.rb
Expand Up @@ -24,6 +24,8 @@ class RuleSet
['border-width', %w[border-top-width border-right-width border-bottom-width border-left-width]]
].freeze

WHITESPACE_REPLACEMENT = '___SPACE___'

class Declarations
class Value
attr_reader :value
Expand Down Expand Up @@ -357,6 +359,7 @@ def expand_dimensions_shorthand! # :nodoc:
#
# TODO: rgba, hsl, hsla
value.gsub!(RE_COLOUR) { |c| c.gsub(/(\s*,\s*)/, ',') }
value.gsub!(RE_FUNCTIONS) { |c| c.gsub(/\s+/, WHITESPACE_REPLACEMENT) }

matches = value.strip.split(/\s+/)
Copy link
Contributor

Choose a reason for hiding this comment

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

would this be better ?

  • keep the replacement close together
  • replace less if all have the same values
  • do not produce extra hash and string objects
Suggested change
value.gsub!(RE_FUNCTIONS) { |c| c.gsub(/\s+/, WHITESPACE_REPLACEMENT) }
matches = value.strip.split(/\s+/)
value.gsub!(RE_FUNCTIONS) { |c| c.gsub!(/\s+/, WHITESPACE_REPLACEMENT); c }
matches = value.strip.split(/\s+/)
matches.each { |c| c.gsub!(WHITESPACE_REPLACEMENT, ' '); c }

... and ideally wrap this up in a preserve_whitespace_in_functions method

Copy link
Contributor Author

Choose a reason for hiding this comment

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

New version pushed, using #split_value_preserving_function_whitespace if that name is okay. How's this look?


Expand All @@ -375,7 +378,10 @@ def expand_dimensions_shorthand! # :nodoc:
end

t, r, b, l = values
replacement = {top => t, right => r, bottom => b, left => l}

replacement = {top => t, right => r, bottom => b, left => l}.transform_values do |replacement_value|
replacement_value.gsub(WHITESPACE_REPLACEMENT, ' ')
end
Copy link
Contributor

Choose a reason for hiding this comment

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

we don't need this then right ?


declarations.replace_declaration!(property, replacement, preserve_importance: true)
end
Expand Down
48 changes: 48 additions & 0 deletions test/test_rule_set_expanding_shorthand.rb
Expand Up @@ -296,6 +296,54 @@ def test_expanding_important_shorthand_with_replaced_properties
assert_equal expected_declarations, declarations
end

def test_functions_with_many_spaces
shorthand = 'margin: calc(1em / 4 * var(--foo));'
declarations = expand_declarations(shorthand)
expected_declarations = {
'margin-top' => 'calc(1em / 4 * var(--foo))',
'margin-bottom' => 'calc(1em / 4 * var(--foo))',
'margin-left' => 'calc(1em / 4 * var(--foo))',
'margin-right' => 'calc(1em / 4 * var(--foo))'
}
assert_equal expected_declarations, declarations
end

def test_functions_with_no_spaces
shorthand = 'margin: calc(1em/4*4);'
declarations = expand_declarations(shorthand)
expected_declarations = {
'margin-top' => 'calc(1em/4*4)',
'margin-bottom' => 'calc(1em/4*4)',
'margin-left' => 'calc(1em/4*4)',
'margin-right' => 'calc(1em/4*4)'
}
assert_equal expected_declarations, declarations
end

def test_functions_with_one_space
shorthand = 'margin: calc(1em /4);'
declarations = expand_declarations(shorthand)
expected_declarations = {
'margin-top' => 'calc(1em /4)',
'margin-bottom' => 'calc(1em /4)',
'margin-left' => 'calc(1em /4)',
'margin-right' => 'calc(1em /4)'
}
assert_equal expected_declarations, declarations
end

def test_functions_with_commas
shorthand = 'margin: clamp(1rem, 2.5vw, 2rem)'
declarations = expand_declarations(shorthand)
expected_declarations = {
'margin-top' => 'clamp(1rem, 2.5vw, 2rem)',
'margin-bottom' => 'clamp(1rem, 2.5vw, 2rem)',
'margin-left' => 'clamp(1rem, 2.5vw, 2rem)',
'margin-right' => 'clamp(1rem, 2.5vw, 2rem)'
}
assert_equal expected_declarations, declarations
end

protected

def expand_declarations(declarations)
Expand Down