-
-
Notifications
You must be signed in to change notification settings - Fork 50
Compatibility matrix redesign for new platforms #3807
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
Conversation
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
@coderabbitai please review |
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed:
Length of output: 6671 🏁 Script executed:
Length of output: 4384 🏁 Script executed:
Length of output: 5225 🏁 Script executed:
Length of output: 16189 @daveverwer I've analyzed PR #3807 which redesigns the compatibility matrix for new platforms. Here's my review: This is a substantial redesign that consolidates the compatibility matrix view to save vertical space by moving platform and Swift version names into the cells. The PR includes many SCSS changes and template updates to support this new design. Regarding the issue mentioned by finestructure about branch icon alignment - I can confirm there appears to be a minor visual issue with the vertical alignment of branch icons in dark mode. The icon appears to have a slightly different baseline alignment compared to the current version. This is likely due to CSS positioning differences in the new design. You may want to check the Overall, the matrix redesign looks good, with this small alignment issue being the only visual concern. |
People can click into *anywhere* on that area and immediately see that builds are pending. I think this is overkill.
b037028
to
05b38d7
Compare
bf605d0
to
930edf2
Compare
It was from your own screenshot above actually |
Where you said “I went with his instead”, but perhaps that wasn’t the latest version? |
Ah, that was to demonstrate the layout I went with and didn't have the other fix applied. |
There are a few options with the matrix redesign. I wanted to combine the header with the compatibility information and have moved the platform and Swift version names inside the cells, coloured with the existing colours:
Unfortunately, it’s not very clear the difference between a pending/unknown state and a failed state for someone who is unfamiliar with the site. We could do something like this:
Alternatively, there is a version in
f2945ee05e91d19fd2d004cee9d32d7bbb4d51ad
that keeps the headers separated, and looks like this:Unfortunately, that takes up significantly more vertical space, especially when compatibility does not match between versions. While it’s nice and clear, it pushes the README way too far down.
Other Improvements
This redesign also changes how we label the versions above the matrices. They’re now separated with a “/” instead of a sentence and it copes with extremely long tag names by truncating the longest tag when they don’t fit on one line: