Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
Test failures seem unrelated. |
📝 WalkthroughWalkthroughThis pull request modifies logo configuration across three files to standardise sizing constraints. The OSS_PARTNERS array receives new maxHeight properties for Nuxt, Vue, and Vue Data UI entries. The SPONSORS array replaces the normalisingIndent field with maxHeight for Vercel, Void Zero, vlt, and Bluesky entries, whilst removing normalisingIndent from Netlify entirely. The LogoList component's BaseItem type signature is updated to use maxHeight instead of normalisingIndent, with corresponding style bindings and layout adjustments to padding and height values. Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 1✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
They had fixed sizes specifically for consistency in effects. When one element has a hover height of 60px, and the next one has a hover height of 40px, it looks like a bug (and is a bit inconvenient for navigation). To compensate for this alignment, normalizingIndent was added to the logo settings. The voidzero group is a special case, since this appearance and such indentations are a strict correspondence to how this group was styled by the authors of the project Maybe we should make the vertical height bigger for all items (equal for each, but bigger, f.e. not h-11 but h-15), but about the other edits - it seems to me that this is trading one problem for another P.S. A few tips: create new branches instead of working in main - this will make future support easier for both you and the maintainers. Also, use |
Okay! I'll leave this up to your judgement. For me, it's much more important for each logo to look good together with its hover background, since, when hovering, they are both visible at the same time. It's the near-collision between the logo and the edge of the hover background that jumps out as a bug to me personally. It's true that my change makes the hover background of the different logos differently sized, but in my opinion this is more difficult to notice, because no two hover backgrounds are visible at the same time. |
🔗 Linked issue
N/A
🧭 Context
Currently, the logo grid on the about page has some issues.
First, the hover background is uneven, and in some parts is close to colliding with the logos. In particular, look at the lack of horizontal spacing on the Vercel logo, and the general lack of spacing on the Open Source Pledge logo.
Screenshots
Screenshots
I added more padding to all of the logos.
I replaced the previous per-logo
padding-blockadjustment with a more reliable per-logomax-heightadjustment. This makes it easier to make the logos look consistent with each other.I added a negative horizontal margin to the logo container, so that logos are flush with the left and right edges.
I generally adjusted all dimensions to make the logos look better.
cc @alexdln 🙏🏻
Before and after, no hover
Before
After
Before and after, with hover background (all hover effects enabled for illustration)
Before
After
Before and after on mobile
Before
After