netlify[bot]
commented
3 weeks ago Deploy Preview for goattack ready!
| Name | Link |
|---|---|
| Latest commit | |
| Latest deploy log | https://app.netlify.com/sites/goattack/deploys/6664f7d228a7e0548846b3a3 |
| Deploy Preview | https://deploy-preview-119--goattack.netlify.app |
| Preview on mobile | Toggle QR Code...Use your smartphone camera to open QR code link. |
Lighthouse |
1 paths audited Performance: 95 (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.
tomtseng
AdamGleave
Fixes #116 Issue:
This PR makes move links jump to the first game (other games can be specified, but we aren't currently specifying any other games). In
content.ts, instead of writingwe write
where
game=game_numberis optional (defaults togame=0)Notes on rationale for implementation
communicating state changes
We previously modified move numbers by exposing a function
window.setMoveat global scope that modifies each section's Go playerwgoPlayer. For this PR, we want move links to not only modifywgoPlayerstate but to also modify Svelte state —selectedRowin<GameList>.Strictly speaking I think we could still solve this by similarly having every
<GameList>save theirselectedRowin a global object and then exposing a globalwindowfunction to modify eachselectedRow, but this isn't how you're "supposed" to communicate Svelte state. So part 1 of this PR is just refactoring so as to not communicate via global state:setContextin a component defines variables visible to its child components. We add this in<Section>to define variables shared between child components<GameList>+<GameBoard>SectionDescriptionwhich parses the descriptions incontent.tsto insert new component<GameMoveLink>that also uses the shared variables in order to communicate with<GameList>and<GameBoard>.this involves some wacky regex parsing — if we'd prefer to just pass around globally scoped variables instead of weird regex I can revert this part
implementing the state change
After the refactor, how do we actually implement updating the game and move, i.e., what is
<GameMoveLink>'sonClickfunction?selectedRow, which reactively triggerswgoPlayer.loadSgfFromFile(sgf), and then callwgoPlayer.goTo(move). This doesn't work becausewgoPlayer.loadSgfFromFile()is async and might not be done by the timewgoPlayer.goTo()is called. Even if it weren't async, it would cause some flickering as we first loaded the SGF and afterwards jumped to the move.wgoPlayer.loadSgfFromFile(sgf, move), but if the SGF doesn't change, then we want to still callwgoPlayer.goTo(move)(we could callwgoPlayer.loadSgfFromFile(sgf, move)every time but it incurs a little extra latency due to re-parsing the SGF). This is complex enough that it's messy to just rely onselectedRowreactively triggering updateswgoPlayer. Instead we write a new functionupdateGame()and explicitly call it everywhere we want to update the game state, which means we need to make a few modifications throughout<GameList>.An alternative implementation I tried: instead of giving
MoveGameLinkthe callbackupdateGame(), let it modify a shared tuple[selectedRow, moveNumber]and then reactively updatewgoPlayerbased on that state. This doesn't work because we can't actually syncmoveNumberwithwgoPlayer(i.e., say we click a move link to changemoveNumberto 1, then we manually use thewgoPlayerUI to change the move number to 4, then click the move link again —moveNumberwas 1 the whole time so no reactive update will be triggered)(We could've based this branch on
maininstead oftomtseng/defense-site, but basing ontomtseng/defense-sitesaves us the annoyance of having to deal withcontent.tsmerge conflicts when both this PR and PR #99 merge. Also, issue #116 is only relevant ontomtseng/defense-siteanyway —maincurrently doesn't have any multi-game sections with move links, which is what causes #116.)