qianzhong516
Repos
25
Followers
6
Following
6

Events

A few suggestions!

Re your fifth point, I wrote an online multi-player GoBang game a year ago. Will share the code if you are interested, but can't guarantee the code quality 🤣 . Feel free to leave your comments in that repo as always. Will attach the link here soon.

Created at 1 day ago
A few suggestions!

Re your fourth point, yep, I'm gonna have a rules button on the bottom corner and link a modal to it showing the rulesets :)

Created at 1 day ago
A few suggestions!

Re your third point. Sounds good because it makes the rolling effect look slightly easier. So, what I can do is

useEffect(() => {
  if (gameStage === GameStage.HOUSE_PICKED) {
  const id = setInterval(() => {
	  setCurrentOpponentAction((prev) => {
		  const nextIndex =
			  (game.shuffledActions.indexOf(prev) + 1) %
			  game.actions.length;
		  const currentAction = game.shuffledActions[nextIndex];
  
		  // let all the actions loop by one cycle
		  if (
			  currentAction ===
			  game.shuffledActions[game.actions.length - 1]
		  ) {
			  setIsAnimationEnded(true);
		  }
  
		  // stop at the right action in the next cycle
		  if (isAnimationEnded) {
			  clearInterval(id);
			  goToGameResultStage();
		  }
		  return currentAction;
	  });
  }, 200);
  
  return () => clearInterval(id);
  }
  }, [...]);

And call game.shuffleActions(); in the onActionIconClick handler.

The problem I'm having now is to get rid of the initial values in player and opponent action state. I think I kept them for walking around some sort of type issues. I will have a look later!

Created at 1 day ago
A few suggestions!

Regarding your second point, yes definitely. PLAYER_PICKED exists because pre the rolling effect was added, there was a stage showing the player picked action against an empty opponent action state before the opponent's action was computed.

Created at 1 day ago
A few suggestions!

Hey @lee-janice, regarding your point 1, it's an indeed innovative idea! I kinda like it. Here is the commit to make it happen. However, I found this solution to be somewhat less intuitive to comprehend, also people who come across this code need to wrap their head around the math. I haven't seen any good reason that we need to switch from the current approach to that one, but do let me know if you want to have any sentiment on the implementation :)

Created at 1 day ago
qianzhong516 create branch enum-action
Created at 1 day ago
opened issue
We should try our best not to disable strict mode rendering in React

I'd like to call out the implications of disabling the React strict mode. Referencing to this excerpt on React official website.

React offers a “Strict Mode” in which it calls each component’s function twice during development. By calling the component functions twice, Strict Mode helps find components that break these rules.

That's said, if you've noticed bugs due to strict-mode rendering, it indicates that you have impure components causing unpredictable bugs. We should make sure the glitch is only stemmed from the vis API itself (which I doubt), not the code we wrote.

https://github.com/lee-janice/wikigraph/blob/88c132999084523c263a2ee1c8e9298b0e3f0a72/src/index.tsx#L9-L11

Created at 1 day ago
Created at 2 days ago
Created at 2 days ago
issue comment
Summary tabs area should be scrollable

@lee-janice I made this game. Feel free to leave issues/comments/questions!

Created at 2 days ago

delete PLAYER_PICKED stage & prolong the rolling effect

Created at 2 days ago
issue comment
Some questions and suggestions

Even though React.createElement and performing diffing algorithm aren't expensive. Whether or not to use optimisation APIs is case-by-case basis. Using profiler is a good idea 👍

Created at 6 days ago
issue comment
Some questions and suggestions

I used the React profiler and it looks to me like the buttons are actually not re-rendering.

Can you show me how you set up the Profiler API? Or are you using the dev tool profiler?

I used the React profiler and it looks to me like the buttons are actually not re-rendering...

Firstly I want to clarify what I meant by re-rendering is calling the render function. Considering this code

<input type="submit" value="Center" id="center-button" /> 

which will be transpiled to

React.createElement("input", {
  type: "submit",
  value: "Center",
  id: "center-button",
});

On each render, React.createElement is called to return a new react element object. However that doesn't necessarily mean the component will be updated in the DOM. React has a diffing algorithm for minimizing the DOM updates by comparing the previous result and the current result.

Now, using memo will directly give you the last rendered result as long as the props aren't changed, so instead of calling React.createElement again, we are using the cached result now.

Created at 6 days ago
issue comment
Some questions and suggestions

every time someone clicks on the vis (which changes the VisContext, which rerenders everything wrapped in the VisContext.Provider)!

This doesn't sound right. We only setVis in App.tsx and vis is supposed to be loaded for once ever since it's the cornerstone of the entire app. I think the reason why vis updates is because summaries and currentSummary updates, which causes App to re-re-render.

Created at 6 days ago
issue comment
Summary tabs area should be scrollable

Imo half-cut tab content is enough to indicate users there is more towards the end, they probably will instinctively scroll or drag to see more, but fade-out effect sounds good to me

Created at 6 days ago
opened issue
Right-click should not select multiple nodes

I'm not a fan of this user experience 🤔 Conventionally ctrl + left click multi-selects things, which is the current behaviour in the app. Right-click to multi-select is confusing as it's hard to find a way to deselect a node. https://github.com/lee-janice/wikigraph/blob/f2f175128868c50eacb1472551d55e3b51d28c54/src/components/wikigraph.tsx#L154

Created at 1 week ago
opened issue
Some questions and suggestions
  1. Consider using React.memo on static components since contextMenuState updates often, which causes unnecessary re-rendering. Below is an example https://github.com/lee-janice/wikigraph/blob/f2f175128868c50eacb1472551d55e3b51d28c54/src/components/wikigraph.tsx#L247-L257
  2. Let's move these handlers in useEffect to a util function. It can make the file look a lot cleaner https://github.com/lee-janice/wikigraph/blob/f2f175128868c50eacb1472551d55e3b51d28c54/src/components/wikigraph.tsx#L103
  3. Is this line doing anything else other than initializing an empty array? https://github.com/lee-janice/wikigraph/blob/f2f175128868c50eacb1472551d55e3b51d28c54/src/components/wikigraph.tsx#L75-L77
  4. Following on the above point, correct me if I'm wrong - selection is not being used for any re-rendering purpose, maybe we don't need to juggle between selection and its ref. Keeping the ref only is fine.
Created at 1 week ago
opened issue
Summary tabs area should be scrollable

More tabs are hidden on the right

image

Created at 1 week ago

implement suggestions

let sidebar use react context for vis and visNetwork

update selection state on select event

Merge pull request #1 from qianzhong516/main

Some suggestions on refactoring

Merge pull request #2 from lee-janice/qianzhong516-main

Merge changes

fix typing for deploy

typing?????

fix light mode omg

fix light mode omg

Created at 1 week ago
issue comment
Some suggestions on refactoring

@qianzhong516 yay, thank you so so much!

R.E. the data, I actually uploaded it into Neo4j/Neo4j AuraDB myself using the WikiMedia clickstream dump! I wanted to use the whole Wikipedia dump but it was 19GB...

I did a bit of pre-processing with Python and I had to cut a bunch of nodes to host it for free 🥹 If you're interested in the process and doing it yourself I can write up a how-to with the cypher commands/command-line things I had to run, I was planning on doing that anyways!

That sounds fun! Though I've never used Neo4J and I'm not quite familiar with Python either but happy to take a look!

Created at 2 weeks ago
issue comment
Some suggestions on refactoring

@lee-janice Thanks for making your effort! I will take a look again on the weekend and think about other improvements if I have time. Like the idea of your project and we have the same name😆. Where did you get the Neo4j dataset from? I looked up on their website but didn't find a wiki example dataset.

Created at 2 weeks ago
pull request opened
Some suggestions on refactoring

Steps I have taken

  1. Managing vis in global state with React.Context
  2. Managing third-party API calls in one place
  3. Adding a loading state if vis is not ready

Reasoning

  1. It shouldn't be WikiGraph's concern on how to set up the vis instance
  2. See comments in the API file
  3. The whole app depends on if vis is loaded
Created at 3 weeks ago
Created at 3 weeks ago
Created at 3 weeks ago
qianzhong516 create branch main
Created at 1 month ago