AlignmentResearch / KataGoVisualizer

MIT License
3 stars 1 forks source link

Go defense website #99

Closed tomtseng closed 2 weeks ago

tomtseng commented 2 months ago

Update our paper website to incorporate results from defense paper.

netlify[bot] commented 2 months ago

Deploy Preview for goattack ready!

Name Link
Latest commit 441272ae9e5a512f9e8d27a5ae57b7dadcb2a4c9
Latest deploy log https://app.netlify.com/sites/goattack/deploys/6671d20754676b000846b216
Deploy Preview https://deploy-preview-99--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 (no change from production)
Accessibility: 95 (🔴 down 2 from production)
Best Practices: 100 (no change from production)
SEO: 100 (🟢 up 8 from production)
PWA: -
View the detailed breakdown and full score reports

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

AdamGleave commented 1 month ago

Hey Tom, excited to see this PR come together! :)

Not had a chance to look at code yet, but some initial thoughts from looking at the deploy preview.

Drop-down NavBar: looks and works great

Logos The navbar with the logos and link to paper/blog/video/etc is quite busy now. Worth thinking if there's any we can cut either the total # of elements, or just make them take up less space? Low-hanging fruit here would be to replace "Attack video" with something less wide (and more in keeping with the other logos) rather than a screenshot. E.g. here's a video icon from NounProject (brand colored version attached) or FlatIcon noun-video-2587221-009999. We could probably also remove GitHub and just include a link to the source code in the footer but that may not be necessary.

Game Moves: Clicking on hyperlinks for go moves update the board but don't scroll down. I think that's the right call but it did mean they seemed like no-ops at first when there's a lot of text. Maybe we could hide the unused table rows (e.g. have it be collapsible under a "see more") for these sections to save a bit of screen space? This is definitely a nice-to-have though, only worth addressing once other more pressing things are done.

Todos Some things to not forget to address before merging:

tomtseng commented 1 month ago

Addressed most comments, still not done with:

Maybe we could hide the unused table rows (e.g. have it be collapsible under a "see more") for these sections to save a bit of screen space?

Addressed by just moving the table below the go board, although I also wrote some code from toggling the table (#114)