-
-
Notifications
You must be signed in to change notification settings - Fork 251
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
Improving the Playground #970
Conversation
@LeoLeBras is attempting to deploy a commit to the ReScript Association Team on Vercel. A member of the Team first needs to authorize it. |
@LeoLeBras, thanks for your PR! I'm a bit surprised though, what browser and extensions do you use? I tested it on Firefox, Chrome and Safari on both desktop and mobile and always get white text on midnight blue which looks pretty readable to me: Even though I really like tailwindcss (and use it in most of my projects), I don't know if we should introduce something not standard here, so we could maybe use some CSS-in-JS if we really want to introduce some custom styles. I'll try to get some feedback about it. |
ok, I found it, it does get displayed in black on machines set to use the light theme, nice catch @LeoLeBras! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for the catch, but the fix is actually simpler, I would rather avoid adding more technologies to this already complex beast.
If you want, you can keep your suggestion in the default code and add the suggestion I wrote.
@LeoLeBras would you update the PR with the changes that @tsnobip suggested? |
I'll do it next week! |
ca45091
to
158505e
Compare
Which OS and browser are you using? @joshderochervlk-simplisafe |
let since_10_1 = `@@jsxConfig({ version: 4, mode: "automatic" }) | ||
module CounterMessage = { | ||
let since_10_1 = `module CounterMessage = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not straightforward for a beginner to understand @@jsxConfig({ version: 4, mode: "automatic" })
. I don’t think it introduces any regressions, but could you confirm that @tsnobip ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, remove it.
@@ -1,7 +1,7 @@ | |||
let css = `body { | |||
background-color: inherit; | |||
color: CanvasText; | |||
color-scheme: light dark; | |||
color-scheme: dark; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
works like a charm
Screen.Recording.2025-02-28.at.09.40.18.mov
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks great, thanks a lot @LeoLeBras
I couldn't repro it either @joshderochervlk-simplisafe, we could fix that in another PR once we have more info. |
No description provided.