added forms for variant game options panes, fixed size bug in polygonal board helper #92
Conversation
|
I haven't read the code yet, but I just tried it out, and looks great! Regarding "All checks have failed": |
| const StartTile: PolygonalTile = new PolygonalTile(new Vector2D(0, 0)); | ||
| const Tiles: PolygonalTile[] = [StartTile]; | ||
| let tileQueue: PolygonalTile[] = [StartTile]; | ||
|
|
||
| for (let i = 1; i < (size + 1) / 2; i++) { | ||
| for (let i = 1; i <= (size - 1) / 2; i++) { |
There was a problem hiding this comment.
Is this a breaking change? Will this affect already-created polygonal boards?
There was a problem hiding this comment.
Good point, yes I believe this would potentially break existing games with polygonal boards.
The "bug" this change tries to fix is not really a problem actually. I just think that the size of the polygonal board grows more naturally with this.
There was a problem hiding this comment.
Okay good to know, breaking changes are okay - we don't have users to be bothered, just wanted to check!
| </select> | ||
| </div> | ||
| <label>Config: </label> | ||
| <template v-if="variant==='baduk' || variant==='phantom' || variant=='capture'"> |
There was a problem hiding this comment.
Just a thought..
It might nice if we could have a mapping from Variants to ConfigViews, kind of like how we map variants to boards (board_map.ts). The benefits would be brevity and separation of concerns.
But that is not a blocker, I think it looks pretty good as-is!
There was a problem hiding this comment.
ok I added a map from variants to the form components, named config_form_map
also ran yarn lint and removed sign restriction from komi inputs.
| .column { | ||
| display: flex; | ||
| flex-direction: column; | ||
| } |
There was a problem hiding this comment.
Somehow I don't like the name of this class. A column can also appear in a grid layout or table layout and those will probably appear in the main.css too. We can leave it like this for now, but maybe I should look up some good practices for naming CSS classes.
There was a problem hiding this comment.
renamed to config-form-column
| style="width: fit-content" | ||
| /> | ||
| </div> | ||
| </template> |
There was a problem hiding this comment.
Why not just add a CSS rule input { width: fit-content } inside a <style> block for the component? Don't repeat yourself (DRY).
Or .awesomeClassName input { width: fit-content } somewhere for all config components?
There was a problem hiding this comment.
added style block and rule
| @@ -0,0 +1,74 @@ | |||
| <script setup lang="ts"> | |||
There was a problem hiding this comment.
Dear Git, why don't you recognize this file isn't new, but was just moved and modified?
See How to make git mark a deleted and a new file as a file move? if you want to avoid this in the future.
There was a problem hiding this comment.
Haha the diff is pretty significant, may have been unavoidable 😅
Edit: well maybe not- I guess 20 lines added
| </script> | ||
|
|
||
| <template> | ||
| <div class="column"> |
There was a problem hiding this comment.
Maybe this <div> could be a <form> with a @change, then it wouldn't be necessary to add @change to each <input>.
No description provided.