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

fix: isPseudoElement() supports :first-letter and :first-line #256

Merged
merged 1 commit into from
Mar 30, 2022
Merged

fix: isPseudoElement() supports :first-letter and :first-line #256

merged 1 commit into from
Mar 30, 2022

Conversation

ybiquitous
Copy link
Contributor

Browsers supports also :first-letter and :first-line pseudo-elements introduced in CSS2.

@alexander-akait
Copy link
Collaborator

@ybiquitous hm, so we parse them as pseudo elements? I think it is bug (for before and fater), because we should parse them as is (i.e. no own decisions), if it was written as pseudo elements we should return pseudo element and versa vice, otherwise linters and another tools, can't understand how developer really write them

@ybiquitous
Copy link
Contributor Author

@alexander-akait Thanks for the quick feedback.

Do you mean it's unnecessary to check :before or :after in the first place like this?

export function isPseudoElement (node) {
    return isPseudo(node) && node.value && node.value.startsWith("::");
}

I don't know the background that isPseudoElement was introduced, so I don't have a strong opinion about these type-guard functions.
Surely, postcss-selector-parser has a Pseudo node type, but doesn't have a PseudoElement or PseudoClass node type.

@alexander-akait
Copy link
Collaborator

Surely, postcss-selector-parser has a Pseudo node type, but doesn't have a PseudoElement or PseudoClass node type.

Oh... that is bad, I messed it

I see what you want, ideally we should rename function to isLegacyPseudoElement, but it will be breaking change, so let's left the same name

Copy link
Collaborator

@alexander-akait alexander-akait left a comment

Choose a reason for hiding this comment

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

Ping me tomorrow, I will do release (I relocated to another city and small busy today)

@ybiquitous
Copy link
Contributor Author

@alexander-akait Don't mind. Thank you for the review. ☺️
I'm not hurrying. Feel free to merge and release when you can afford it.

@alexander-akait alexander-akait merged commit 7f450cb into postcss:master Mar 30, 2022
@alexander-akait
Copy link
Collaborator

Thanks

@alexander-akait
Copy link
Collaborator

@ybiquitous ybiquitous deleted the fix-guard-for-first-letter-first-line branch March 30, 2022 23:53
@ybiquitous
Copy link
Contributor Author

@alexander-akait Thank you! 🎉

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.

None yet

2 participants