-
-
Notifications
You must be signed in to change notification settings - Fork 39
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
Extended HSL/HSLA/HSB Handling with Float Support #93
Conversation
src/HsPatterns.php
Outdated
|
||
class HsPatterns | ||
{ | ||
private const HUE = '\d{1,3}'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's make these protected
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These also can be regular class properties, not constants.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I decided to change from private
to protected
.
My initial idea was to create a ColorPattern
class instead of HsPattern
, but that would have required a complete refactor of the package—and doing that the day before Christmas would have been a bit naughty.
Using constants feels cleaner to me, as it allows me to avoid touching VALIDATION_PATTERNS
and EXTRACTION_PATTERNS
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's make these regular properties, they can be static
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi Freek,and thanks for the suggestion! I'm a bit confused, I understand the reasoning, but I think constants are a better fit in this specific case:
- Immutability: These patterns (HUE, COMPONENT, ALPHA) are not meant to change at runtime, so using constants makes the intent clearer and avoids unnecessary complexity. 🤓
- Performance: Constants are resolved at compile time, while static properties involve runtime allocation.
- PHP Compatibility: Since the composer.json specifies PHP 7.3 as the minimum version, using typed static properties (introduced in PHP 7.4) would break compatibility.
If I misunderstood your comment, I’m here to help and follow your directions. Please feel free to double-check how I’ve declared the constants in the file to ensure they align with your expectations!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, those are good arguments, thanks for explaining. 👍
Thank you! |
This PR introduces extended support for handling HSL, HSLA, and HSB color formats with floating-point values.
The changes address and resolve Issue #89.
Changes:
Centralized Regex Patterns:
HsPatterns
class to centralize and manage regex patterns for validation and extraction.Enhanced Parsing Logic:
Hsb
,Hsl
, andHsla
classes to use the newHsPatterns
class for parsing.fromString
methods to correctly parse and handle floating-point values in color strings.Validation Improvements:
hsbColorString
,hslColorString
, andhslaColorString
) to utilize the new centralized regex patterns inHsPatterns
.Tests and Dataset Enhancements:
Resolves:
Issue #89