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

Improve Code Rendering #36

Merged
merged 2 commits into from
Jan 14, 2025
Merged

Conversation

irfanfadilah
Copy link

@irfanfadilah irfanfadilah commented Jan 14, 2025

This PR improves the style for block and inline <code> tags.

Fix Overflow

Commit: 37dac63

Move the overflow-x: auto; property from pre to code to fix scrolling and padding.

Before After
pre-code-before pre-code-after

Improve Inline Spacing

Commit: 3946e8f

The inline <code> tags touch each other when stacked in multiple lines, so I adjusted the vertical padding to 0.125rem. I also added vertical-align: middle; to improve the text alignment.

Before After
inline-code-before inline-code-after

@irfanfadilah
Copy link
Author

Hi @Yohn, this is the mirror PR of picocss#651.

However I'm getting these errors when trying to build it:

Error: Undefined function.
  ╷
7 │     "rgb(" + color.channel($color, "red", $space: rgb) + ", " +
  │              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  ╵
  scss/helpers/_functions.scss 7:14         display-rgb()
  scss/themes/default/_styles.scss 106:171  @use
  scss/themes/_default.scss 2:1             @use
  scss/_index.scss 7:1                      @use
  scss/pico.classless.scss 2:1              root stylesheet

Error: Undefined function.
   ╷
28 │       (color.channel($color, "red", $space: rgb) * 299) +
   │        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   ╵
  scss/colors/utilities/_utils.scss 28:8               brightness()
  scss/colors/utilities/_utils.scss 12:33              foreground-brightness()
  scss/colors/utilities/_background-colors.scss 14:9   foreground-color()
  scss/colors/utilities/_background-colors.scss 67:15  background-colors()
  scss/colors/utilities/_index.scss 8:1                @use
  scss/pico.colors.scss 2:1                            root stylesheet

Error: Undefined function.
  ╷
7 │     "rgb(" + color.channel($color, "red", $space: rgb) + ", " +
  │              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  ╵
  scss/helpers/_functions.scss 7:14         display-rgb()
  scss/themes/default/_styles.scss 106:171  @use
  scss/themes/_default.scss 2:1             @use
  scss/_index.scss 7:1                      @use
  scss/pico.conditional.scss 2:1            root stylesheet

Error: Undefined function.
  ╷
7 │     "rgb(" + color.channel($color, "red", $space: rgb) + ", " +
  │              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  ╵
  scss/helpers/_functions.scss 7:14               display-rgb()
  scss/themes/default/_styles.scss 106:171        @use
  scss/themes/_default.scss 2:1                   @use
  scss/_index.scss 7:1                            @use
  scss/pico.fluid.classless.conditional.scss 2:1  root stylesheet

Error: Undefined function.
  ╷
7 │     "rgb(" + color.channel($color, "red", $space: rgb) + ", " +
  │              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  ╵
  scss/helpers/_functions.scss 7:14         display-rgb()
  scss/themes/default/_styles.scss 106:171  @use
  scss/themes/_default.scss 2:1             @use
  scss/_index.scss 7:1                      @use
  scss/pico.fluid.classless.scss 2:1        root stylesheet

Error: Undefined function.
  ╷
7 │     "rgb(" + color.channel($color, "red", $space: rgb) + ", " +
  │              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  ╵
  scss/helpers/_functions.scss 7:14         display-rgb()
  scss/themes/default/_styles.scss 106:171  @use
  scss/themes/_default.scss 2:1             @use
  scss/_index.scss 7:1                      @use
  scss/pico.scss 2:1                        root stylesheet
ERROR: "build:css" exited with 65.

@Yohn
Copy link
Owner

Yohn commented Jan 14, 2025

I believe those errors were the sass errors from the original pico repo, you shouldn't have that issue when building from this fork.
Did you get those errors for the original pico build?

Also, I setup a workflow that automatically builds the css directory so no need to run the build command before submitting a pull request here.

Thanks for submitting the pull request! It looks like it's passing, I'll check it out more when I get back to my computer.

@irfanfadilah
Copy link
Author

This PR is based on your main branch and it is also returning the errors on the main branch.

Did you get those errors for the original pico build?

I can build without any issues on the original Pico repo.

I setup a workflow that automatically builds the css directory so no need to run the build command before submitting a pull request here.

That's pretty cool! Contributors don't need to manually build the files, it is also make code review cleaner. You may also want to put this note to the contributing guidelines.

@Yohn Yohn merged commit 8b09efb into Yohn:main Jan 14, 2025
1 check failed
@Yohn
Copy link
Owner

Yohn commented Jan 14, 2025

Merged correctly.

I saw a build thing here in github to build before merging, and I tried that and it failed - but I think its because of the github token being on my account, and not yours. Thats why I'm seeing a red X by this request.

After I merged it, the workflow went a built the css files as expected and it worked out correctly.
This will be included in the next release.

What version of sass do you have installed? I just checked mine and got:

sass --version
1.81.0 compiled with dart2js 3.5.4

and I see the current version is 1.83.4 so I'll be updating shortly to see if I find the same error messages.

And the contributing guidelines. have been updated! Thanks for pointing that out!

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