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

Add more css properties, shorthands #258

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

lucyxiang
Copy link

@lucyxiang lucyxiang commented Jan 19, 2023

Noticed that ACCEPTABLE_CSS_PROPERTIES is missing properties so adding some more (non exhaustive).

There’s a hodge podge of reasons why I choose these properties: I read through a bunch of “most common css properties / shorthands articles" and guesstimated what would be popular

@lucyxiang lucyxiang force-pushed the add-additional-acceptable-css-properties branch from 568043e to a006f1e Compare January 19, 2023 21:24
@flavorjones flavorjones self-requested a review January 19, 2023 21:29
@lucyxiang lucyxiang force-pushed the add-additional-acceptable-css-properties branch from a006f1e to e224b62 Compare January 19, 2023 21:31
@flavorjones
Copy link
Owner

@lucyxiang There are some failing tests because the existing shorthand CSS properties logic appears to have a bug that I'll need to dig into.

Can you break this up into two pull requests: one for the properties and keywords (which I think can be merged quickly), and another one for the additional shorthand properties (which I'll use to debug the failing tests)?

@lucyxiang lucyxiang force-pushed the add-additional-acceptable-css-properties branch from e224b62 to d386bb2 Compare January 23, 2023 16:06
@flavorjones
Copy link
Owner

These properties and keywords look OK! There is one failing test, though, which looks like it's failing because the position property was previously disallowed but is now allowed. Are you able to update that test so it passes?

@flavorjones
Copy link
Owner

For Rails Conf 2024 hack day, maybe someone would be willing to put in a little bit more work to get this merged?

@factcondenser
Copy link

@flavorjones I'm continuing this work over in #283

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

Successfully merging this pull request may close these issues.

None yet

3 participants