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

V2 css parser document #164

Open
wants to merge 12 commits into
base: v2
Choose a base branch
from
Open

Conversation

stoivo
Copy link
Contributor

@stoivo stoivo commented Jun 12, 2024

I started to to try to clean up the Parser class. I have more to come, just want to open the MR if so I can get some feedback

Copy link
Contributor

@grosser grosser left a comment

Choose a reason for hiding this comment

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

seems easier to understand 👍
lmk if it's ready to merge

This Comes with another benefit too. It allow users to replace the logic
for how reading url should work. If they don't want to allow http or
follow redirects for 60 seconds.
Can find any use of it
Media queries can be anything so it don't make much sense to have it as
symbol. The media "print" seam like could be nice as a media query but
it can be combined with "max-width: 1px", to "pring and max-width: 1px)"
which make less sense as a symbol.
Instances of CssParser::Parser had little to nothing to do with parsing
the actual css. It's a wrapper to hold the ruleset and give some
convince method on top of that.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants