Modernize ui/learn #15263
Modernize ui/learn #15263
Conversation
|
Impressive. How far are we from the PR being ready for review? |
|
Thanks, life has been quite busy and I haven't had the time to work on it but this is justtt about there. The main remaining issue is with the various chessground shapes all playing nicely together. It seemed like some kind of race condition because the behavior was inconsistent. I think specifically the shape we add when animating a piece capture doesn't always appear. Then there is maybe a few smaller documented TODOs like disabling context menu on chessground (the config option didn't seem to be working). I bet this week I could find sometime to get it over the finish line though. If anyone is eager and wants to go for it, no problem of course! Final note, this doesn't actually remove the custom chess-js stuff, it felt like a big enough change on its own already. It does completely remove the mithril stuff and update chessground. |
|
Nice, with a fresh brain I was able to figure out a fix. I'm going to keep hunting for bugs but I think this could be reviewed now. |
| @@ -63,6 +63,7 @@ public/sounds/nes | [Enigmahack](https://github.com/Enigmahack) | AGPLv3+ | |||
| public/sounds/piano | [Enigmahack](https://github.com/Enigmahack) | AGPLv3+ | |||
| public/sounds/sfx | [Enigmahack](https://github.com/Enigmahack) | AGPLv3+ | |||
| public/sounds/lisp | [EdinburghCollective](http://lichess.org/@/EdinburghCollective) | [CC BY-NC-SA 4.0](https://creativecommons.org/licenses/by-nc-sa/4.0/) | |||
| SVG in ui/learn/src/apple.ts | [Sensa](https://www.svgrepo.com/svg/434273/star) | [CC0 1.0](https://creativecommons.org/publicdomain/zero/1.0/deed.en) | |||
There was a problem hiding this comment.
This feels a bit out of place here, maybe only because it is in a typescript file and not in the public folder. Not sure if there is a better place though?
There was a problem hiding this comment.
Also it is public domain, so maybe not even needed, but feels nice to do
| @@ -18,6 +18,7 @@ export default function (ctrl: RunCtrl): VNode { | |||
| hook: { | |||
| insert: vnode => { | |||
| const el = vnode.elm as HTMLElement; | |||
| el.addEventListener('contextmenu', e => e.preventDefault()); | |||
There was a problem hiding this comment.
We shouldn't need this, since we have disableContextMenu: true,, but that config setting alone wasn't working 🤔
|
That's a humongous work. Thank you! |
Fixes #15098.
Lots of changes here, but essentially feature equivalent to what we have today. I say essentially equivalent only because I found a few things that were broken and fixed those up (just styling things). Major things achieved:
Major failures:
We needed to use a SVG star, so it appears slightly different. Happy to tweak it if desired:
