Skip to content

[TASK] Add SelectorComponent interface - #1478

Merged
oliverklee merged 3 commits into
mainfrom
task/selectorpart-interface
Jan 29, 2026
Merged

[TASK] Add SelectorComponent interface#1478
oliverklee merged 3 commits into
mainfrom
task/selectorpart-interface

Conversation

@JakeQZ

@JakeQZ JakeQZ commented Jan 23, 2026

Copy link
Copy Markdown
Collaborator

This will be implemented by CompoundSelector and Combinator to break up the components of a selector.

Part of #1325.

@JakeQZ JakeQZ self-assigned this Jan 23, 2026
@JakeQZ JakeQZ added enhancement refactor For PRs that refactor code without changing functionality labels Jan 23, 2026
@coveralls

coveralls commented Jan 23, 2026

Copy link
Copy Markdown

Coverage Status

coverage: 71.315%. remained the same
when pulling 37c13b8 on task/selectorpart-interface
into 9641004 on main.

@JakeQZ
JakeQZ force-pushed the task/selectorpart-interface branch from 2eb2b79 to 6562f64 Compare January 23, 2026 23:21
@JakeQZ

JakeQZ commented Jan 23, 2026

Copy link
Copy Markdown
Collaborator Author

I'd like feedback on method names (and also interface name), as once committed it will be harder to change later.

#1471 is an early example of implementation (with different names).

Refs:

Comment thread src/Property/Selector/SelectorComponent.php
Comment thread src/Property/Selector/SelectorComponent.php Outdated
@JakeQZ
JakeQZ requested a review from oliverklee January 25, 2026 19:29
Comment thread src/Property/Selector/SelectorComponent.php
This will be implemented by `CompoundSelector` and `Combinator` to break up
the components of a selector.

Part of #1325.
Change `getStringValue()` to `getValue()` (and also for `setValue()`).
@JakeQZ
JakeQZ force-pushed the task/selectorpart-interface branch from a293ba1 to 2eb9734 Compare January 27, 2026 23:02
@JakeQZ

JakeQZ commented Jan 28, 2026

Copy link
Copy Markdown
Collaborator Author

GitHub won't let me reply to your review comment.

I've changed the DocBlock. Is it now clearer?

@JakeQZ
JakeQZ requested a review from oliverklee January 28, 2026 18:15

@oliverklee oliverklee left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks, that's definitely better!

@oliverklee
oliverklee merged commit f78fca5 into main Jan 29, 2026
24 checks passed
@oliverklee
oliverklee deleted the task/selectorpart-interface branch January 29, 2026 10:43
@JakeQZ

JakeQZ commented Jan 30, 2026

Copy link
Copy Markdown
Collaborator Author

As this is already in the namespace Selector, I am now wondering if the interface should be simply Component, rather than SelectorComponent. The extra Selector in the name seems redundant.

E.g.

use Sabberworm\CSS\Property\Selector\SelectorComponent;

Might look better as

use Sabberworm\CSS\Property\Selector\Component;

@oliverklee WDYT?

@oliverklee

Copy link
Copy Markdown
Collaborator

@JakeQZ Totally makes sense! So I would be fine with dropping the duplicate Selector from the interface name.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement refactor For PRs that refactor code without changing functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants