Conversation
validbeck
commented
Mar 26, 2024
- Adjusted the header background colour & padding
- Adjusted the footer padding
- Original for comparison: https://drive.google.com/file/d/1vi549IwCSXT3Z7midlEAD0XQIHbSSBHV/view?usp=drive_link
- Updated preview: https://drive.google.com/file/d/10cARMybalr9EXFLbah5ysZm0ipIcOcG6/view?usp=drive_link
- Adjusted the footer padding - Original for comparison: https://drive.google.com/file/d/1vi549IwCSXT3Z7midlEAD0XQIHbSSBHV/view?usp=drive_link - Updated preview: https://drive.google.com/file/d/10cARMybalr9EXFLbah5ysZm0ipIcOcG6/view?usp=drive_link
nrichers
left a comment
There was a problem hiding this comment.
What a great first 'real' PR, @validbeck! 🚀 Kudos to finding your way around our docs site so quickly.
Besides the comments I left elsewhere, some additional thoughts before passing this to Mehdi for review:
-
On the landing page, the navbar color change means we end up with this white space. Would it make sense to tweak that, e.g. omit the white space or at least make it the same color as the navbar? Generally, that landing page needs a bit of a makeover, but we should avoid introducing new issues with it.
The source for this page is in index.qmd.
-
With the new navbar background color, the ValidMind logo looks more like a text highlight. The logo hasn't changed from before, but maybe the issue is just more apparent with a new background? We do have a black variant of our logo that might be worth trying out.
The logo gets defined in _quarto.yml#L22.
-
There is some CSS weirdness on the landing page vs the rest of the pages. Specifically, your padding doesn't show up on the landing page. I remember running into something similar ages ago, but I'll be darned if I can remember what the fix was.
Let me know if you can't figure out and I'll see if I can't dust off some of the cobwebs pretending to be my memory.
-
For giggles, I tried changing the "Developer Framework" drop-down as well and I don't think it makes much of a difference. For reference here are what they look like side by side. (If anything, a lighter shade of pink might works as the background.)
| Current | Experiment |
|---|---|
 |
 |
|
|
||
| $navbar-fg: #2e3133; | ||
| $navbar-hl: #de257e; | ||
| $navbar-bg: #FCE9F2; |
There was a problem hiding this comment.
| $navbar-bg: #FCE9F2; | |
| $navbar-bg: #fce9f2; |
(Honestly, just a left eye twitch consistency suggestion. AFAIK, hexadecimal color codes are case-insensitive and all the other colors are in lowercase.)
| font-size: .8em; | ||
| } | ||
|
|
||
| .nav-item { |
There was a problem hiding this comment.
With the padding being introduced here and in other places, the left sidebar navigation is starting to feel a bit squished. Would it maybe make sense to tinker with that a bit as well? It might not need much.
FWIW, I did try a few different screen resolutions and the padding introduced here works fine.
|
One other comment: please update the root comment with the actual previews — you can even include them side by side in a Markdown table (see the source for my review comment). After the PR is approved, please remember to run |
|
FYI, I opened https://app.shortcut.com/validmind/story/4120 to unblock this pull request. Not scheduled for a sprint, yet. |