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

Bugfix - Fixes in CSS generation - "sketch_css" #29

Merged

Conversation

josevictorferreira
Copy link
Contributor

@josevictorferreira josevictorferreira commented Oct 26, 2024

This PR fixes 3 issues with CSS generation using "sketch_css":

  1. Property names are being generated with "_" instead of "-" as separator. Ex: z_index(100) was generated with the property name z_index instead of z-index.
  2. Properties with Int or Float values are being generated with empty values. Ex: opacity(1.0) generated as opacity: ;.
  3. Percent size values being generated with the wrong symbol. Ex: width(percent(100)) being generated as width: 100percent instead of width: 100%.

fn size_to_string(value) {
fn size_to_string(size: String) -> String {
case size {
"percent" -> "%"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I haven't found a better way to do that, since the name of the size function is "percent" and the value for the size variable comes from the AST

Comment on lines +478 to +483
fn size_to_string(size: String) -> String {
case size {
"percent" -> "%"
_ -> size
}
}
Copy link
Owner

@ghivert ghivert Oct 28, 2024

Choose a reason for hiding this comment

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

Maybe you could do something like this, but your version is fine. 🙂

Suggested change
fn size_to_string(size: String) -> String {
case size {
"percent" -> "%"
_ -> size
}
}
fn size_to_string(size: String) -> String {
use <- bool.guard(when: size == "percent", return: "%")
size
}

@ghivert
Copy link
Owner

ghivert commented Oct 28, 2024

Thanks for your PR! I see you're still pushing some fixes, do you think you have additional fixes to do, or is it OK as is?

@josevictorferreira
Copy link
Contributor Author

I think it's OK, those are the only issues I've stumbled upon. The rest of the library is working great!

@ghivert
Copy link
Owner

ghivert commented Oct 31, 2024

Thank you!

@ghivert ghivert merged commit 2d3eae3 into ghivert:main Oct 31, 2024
4 checks passed
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