⚠️ No Changeset found

Latest commit: 1063afa

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Hello! Thank you for opening your first PR to Starlight! ✨

Here’s what will happen next:

1. Our GitHub bots will run to check your changes.
   If they spot any issues you will see some error messages on this PR.
   Don’t hesitate to ask any questions if you’re not sure what these mean!

2. In a few minutes, you’ll be able to see a preview of your changes on Vercel 🤩

3. One or more of our maintainers will take a look and may ask you to make changes.
   We try to be responsive, but don’t worry if this takes a few days.

Lunaria Status Overview

🌕 This pull request will trigger status changes.

Tracked Files

Thanks for your contribution 🙌

I did not get a look at the code yet but from a quick look on the results, this is looking like a great improvement. On top of background color changes, I'm also wondering if some work on border colors could be also beneficial, thinking for example on the case of caution aside in light mode. Do you have any thoughts on that?

I'm planning to take a closer look with more eyes this Thursday during Astro Talking & Doc'ing session on Discord as it's a great opportunity to discuss it with the community and have more eyes on it which is great for visual changes. If you're available, feel free to join us there, it would be great to have you there to discuss it with us.

Thanks for your contribution 🙌

I did not get a look at the code yet but from a quick look at the results, this is looking like a great improvement. On top of background color changes, I'm also wondering if some work on border colors could be also beneficial, thinking for example on the case of caution aside in light mode. Do you have any thoughts on that?

I'm planning to take a closer look with more eyes this Thursday during Astro Talking & Doc'ing session on Discord as it's a great opportunity to discuss it with the community and have more eyes on it which is great for visual changes. If you're available, feel free to join us there, it would be great to have you there to discuss it with us.

@astrobot-houston
Thanks for your review of this PR.
I couldn't have a chance to join the Astro talking on Thursday, but I believe adding a border to styling is a great improvement.
I will work on adding the border as soon as possible.

I couldn't have a chance to join the Astro talking on Thursday

No worries at all. To summarize what happened, we mostly did a visual review, on various OS, browsers, using both the light and dark themes, tried various scenarios, etc. and the conclusion was that the original issue is clearly addressed by your changes and amazing work.

Regarding the borders, this is something that also came up in the discussion, but after trying various options, we think that all table borders may need some work, not only the one in asides. This is a more general issue that we could address in a separate PR after some discussion identifying the best approach, what are the general issues with the current borders, and how we can improve them.

Considering all this, I think the best approach would be to keep the current PR as-is, have a more code oriented review and ship the current changes as they fix the original issue. Then, we can start discussing the general border issue in a separate PR.

What do you think?

No worries at all. To summarize what happened, we mostly did a visual review, on various OS, and browsers, using both the light and dark themes, tried various scenarios, etc. and the conclusion was that the original issue was addressed by your changes and amazing work.

Regarding the borders, this is something that also came up in the discussion, but after trying various options, we think that all table borders may need some work, not only the one in the asides. This is a more general issue that we could address in a separate PR after some discussion identifying the best approach, what are the general issues with the current borders, and how we can improve them.

Considering all this, I think the best approach would be to keep the current PR as-is, have a more code-oriented review, and ship the current changes as they fix the original issue. Then, we can start discussing the general border issue in a separate PR.

What do you think?

I agree with you that we should fix the general styling of the table. I was wondering whether this PR should be shipped the way it is or if it needs some changes on certain parts of the code. If there need to be some minor changes, would it be possible for you to list them so I can try fixing them?

Thank you in advance,

Snowdingo.

Hey all! Thanks so much for digging into this. I took some time to play around with it a bit and think about it and wondered if maybe one approach could be switching to a table design that doesn’t rely on these background colours at all.

For example maybe something like the experiments here:
https://stackblitz.com/edit/github-2qdnrc?file=src%2Ftables.css

Might need some more discussions with team as these styles are quite different, but could be a nice way to both simplify things and solve this issue.