Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expose API via useImperativeHandle. #16

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

isofarro
Copy link

@isofarro isofarro commented Sep 27, 2023

Following on from the suggestion in #14 , reimplemented my changes using a forwardRef and useImperativeHandle. Returns an instance of the Chessground API for the current board.

Sample code demonstrating it's use (added to the README too):

import { useEffect, useRef } from "react";
import Chessground, { Api, Config, Key } from "@react-chess/chessground";

// these styles must be imported somewhere
import "chessground/assets/chessground.base.css";
import "chessground/assets/chessground.brown.css";
import "chessground/assets/chessground.cburnett.css";

const CONFIG: Config = {
  movable: { free: false },
};

// Demo game moves in long algebraic form
const MOVES = (
  "e2e4 e7e5 g1f3 d7d6 d2d4 c8g4 d4e5 g4f3 d1f3 d6e5 " +
  "f1c4 g8f6 f3b3 d8e7 b1c3 c7c6 c1g5 b7b5 c3b5 c6b5 " +
  "c4b5 b8d7 e1c1 a8d8 d1d7 d8d7 h1d1 e7e6 b5d7 f6d7 " +
  "b3b8 d7b8 d1d8"
).split(" ");

export const DemoGameMoves = () => {
  const apiRef = useRef<Api | undefined>();

  useEffect(() => {
    // Make a move every 2 seconds
    const interval = setInterval(() => {
      const move = MOVES.shift();
      if (move) {
        apiRef.current!.move(move.substring(0,2) as Key, move.substring(2,4) as Key);
      } else {
        clearInterval(interval);
      }
    }, 2000);
    return () => clearInterval(interval);
  });

  return (
    <Chessground
        width={640} height={640}
        config={CONFIG} ref={apiRef}
    />
  );
};

@isofarro isofarro mentioned this pull request Sep 27, 2023
@nowszy94
Copy link

nowszy94 commented Sep 28, 2023

Would be nice to have api usage example in README.md

src/index.tsx Outdated
api?.set(config);
}, [api, config]);
useImperativeHandle(apiRef, () => {
return publicApi;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about exposing the whole chessground api instead of creating custom object?

Copy link
Author

@isofarro isofarro Sep 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, we can do that. Done now.

Also re-exported Config, Api and Key from the chessground API to reduce the clutter of imports in code using this React wrapper. Does that sound sensible?

@isofarro
Copy link
Author

Would be nice to have api usage example in README.md

I've added the example demo-game component from this PR description to the README, and added ref to the props documentation.

@@ -40,3 +42,51 @@ import "chessground/assets/chessground.cburnett.css";

ReactDOM.render(<Chessground />, document.getElementById("root"));
```

## Example: showing the moves of a game

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i really like this example!

src/index.tsx Outdated
useEffect(() => {
if (divRef.current && !api) {
const chessgroundApi = ChessgroundApi(divRef.current, {
animation: { enabled: true, duration: 200 },

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lets remove it

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed.

src/index.tsx Outdated
...config,
});
setApi(chessgroundApi);
} else if (divRef.current && api) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it needed? It looks like it can only be fired after line 34 completes and after that the config is already provided

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I gave it a quick test, and you're right, it doesn't look to be needed. Removed.

Copy link

@nowszy94 nowszy94 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fantastic!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants