AlignmentResearch / KataGoVisualizer

MIT License
3 stars 1 forks source link

Website: Refactor navbar and other components with bootstrap #103

Closed tomtseng closed 1 month ago

tomtseng commented 1 month ago

Context

To better organize the website when we add content from the Go defense paper, we want to have a hierarchical navbar. Bootstrap has a navbar component supporting drop-downs. This PR changes our navbar to use Bootstrap.

Using Bootstrap, however, requires importing Bootstrap's CSS, which breaks a bunch of our CSS. In particular, the table of contents no longer floated to the left like it used to, so this PR expanded to fix the table of contents and to do other CSS cleanup that became possible due to having Bootstrap's CSS.

Main changes

I'll leave GitHub comments on other minor changes.

netlify[bot] commented 1 month ago

Deploy Preview for goattack ready!

Name Link
Latest commit 284b380f67bf58bf7b7a4be0511947eaf42659f5
Latest deploy log https://app.netlify.com/sites/goattack/deploys/664006ccbd93a6000832c0cc
Deploy Preview https://deploy-preview-103--goattack.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

Lighthouse
1 paths audited
Performance: 98 (🟢 up 8 from production)
Accessibility: 97 (🟢 up 2 from production)
Best Practices: 100 (no change from production)
SEO: 85 (no change from production)
PWA: -
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify site configuration.

tomtseng commented 1 month ago

The Go viewer buttons render as black squares in Safari, but this is a problem on main as well so shouldn't block merging this PR, but might be worth fixing if easy (I'd timebox it to <1 hour though -- not that many people use Safari and it's usable):

they appear fine for me, if this isn't new to this PR then I'll defer to a separate PR

The only other issue I noticed was the asymmetric Go viewer at certain sizes (replicated on Chrome & Firefox), which I think you already know -- fine to merge without addressing but we can open an issue to track it?

Fixed in #106

Could we increase the contrast of the bullet point list/TOC icon, e.g. just put it on a blue background with a circle? It's very hard to spot sometimes:

blue looked a little intense, I gave it a light gray background and increased the background opacity

AdamGleave commented 1 month ago

Changes to color scheme & increasing contrast to TOC look great to me -- happy for you to merge whenever you want.