Feat/virtual joystick

[JWS-aAI]

New feature added: Basic Virtual Joystick

Hi, I decide implement this feature because it appears to be in high demand. Hope you find interesting.

Description:

• Joystick can be activated by button on bottom (see picture)
• For resolutions less than 650px wide it's active by default
• When resolution ir less than 650px and chat is active, joystick is hidden because it interferes with the chat

For implement this, I use this component: https://github.com/elmarti/react-joystick-component

Note: This feature will conflict with this PR: #.62
I can take care of resolving conflicts

Related issues:

• #.23
• #.60

[PR-Review]

It's good to see that you've integrated the joystick movement with the existing keyboard controls. However, please ensure that the joystickMovement property is always initialized properly to avoid potential null reference issues when accessing its isMoving and direction properties.

[JWS-aAI]

@PR-Review How do you suggest I do this?

[PR-Review]

@JWS-aAI To ensure that the joystickMovement property is always initialized properly and to avoid potential null reference issues, you can take the following steps:

1. Default Initialization: When you declare the joystickMovement property in your MyPlayer class, you can initialize it with a default value that represents the state of no movement. For example:

public joystickMovement: JoystickMovement = {
  isMoving: false,
  direction: {
    left: false,
    right: false,
    up: false,
    down: false,
  },
};

2. Null Checks: Whenever you access the joystickMovement property, perform a null check to ensure it is not undefined before accessing its isMoving and direction properties. This is a defensive programming practice that can prevent runtime errors.

3. TypeScript Non-null Assertion Operator: If you are confident that joystickMovement will always be initialized before any access, you can use the non-null assertion operator (!) when accessing it. However, this should be used with caution and only when you are certain that the property cannot be null at that point in the code.

4. Initialization in Constructor: If the joystickMovement property is meant to be set during the construction of the MyPlayer object, ensure that it is properly initialized in the constructor or immediately after the object is created.

By following these suggestions, you can make your code more robust and prevent issues that might arise from trying to access properties of an undefined object.

[PR-Review]

The integration of joystick input for the player selector is consistent with the changes made in MyPlayer.ts. However, please ensure that the joystickMovement property is checked for null before accessing its properties to prevent any runtime errors.

[JWS-aAI]

@PR-Review this won't be an issue.

[PR-Review]

@JWS-aAI Understood. If you're confident that the joystickMovement property will always be initialized before it's accessed, and you've implemented measures to ensure it won't be null, then it should be fine. Just make sure that this assumption is valid throughout all possible execution paths in your application to avoid any unexpected behavior.

It's always good practice to have safeguards in place, but if your application logic guarantees that joystickMovement will never be null when accessed, then additional null checks may not be necessary. Just keep in mind that any future changes to the code should maintain this guarantee to prevent potential issues.

[PR-Review]

The change in the Backdrop styled component to position the chat higher up from the bottom of the screen is noted. Please ensure that this change does not interfere with other UI elements and that it is consistent with the intended design, especially on different screen sizes.

[PR-Review]

The modification to the close button position in the Wrapper styled component is acknowledged. Please verify that this change does not cause the button to overlap with other UI elements or become less accessible, particularly on smaller screens or different resolutions.

[PR-Review]

The addition of new buttons for enabling and disabling the virtual joystick is a good feature. However, please ensure that the tooltip text and the button icons are intuitive enough for users to understand their purpose. Additionally, consider the accessibility of these buttons and whether they are easily reachable and identifiable within the UI.

[JWS-aAI]

@PR-Review Sorry, could you clarify?

[PR-Review]

@JWS-aAI Certainly! My comment was regarding the new buttons you've added to the HelperButtonGroup.tsx for toggling the virtual joystick on and off. Here are the specific points to consider:

1. Intuitive Tooltip and Icons: Make sure that the icons used for the joystick toggle buttons (such as VideogameAssetIcon and VideogameAssetOffIcon) clearly convey their function to the user. The tooltip text should also be descriptive enough so that users can easily understand what will happen when they click the button.

2. Accessibility and Visibility: The buttons should be placed in a location within the UI that is easy for users to find and click. They should not be obscured by other elements on the screen, and their size should be adequate for interaction, especially on touch devices. Additionally, consider the color contrast and visual cues that help users with different abilities to identify and use these buttons effectively.

By addressing these points, you'll enhance the user experience by making the controls more user-friendly and accessible to a wider audience.

[PR-Review]

The adjustments made to the Backdrop, Wrapper, and WhiteboardWrapper styled components in the WhiteboardDialog are noted. Please ensure that these changes do not affect the layout and usability of the whiteboard feature on various screen sizes. The close button's new position should also be tested for accessibility and to ensure it does not overlap with other UI elements.

[PR-Review]

The addition of the showJoystick state property and the setShowJoystick reducer in the userSlice is appropriate for managing the joystick display state. However, please be aware that accessing window.innerWidth directly in the Redux slice can cause issues during server-side rendering or testing. Consider initializing this state in a component or through a middleware that is client-side specific.

[JWS-aAI]

@PR-Review Could you generate a short test function for this?

[PR-Review]

@JWS-aAI Certainly! To test the showJoystick state property and ensure it's initialized correctly on the client side, you can create a test function that simulates different window widths and checks if the showJoystick state is set as expected. Here's an example of how you might write such a test function using Jest and React Testing Library:

import { render, screen } from '@testing-library/react';
import { Provider } from 'react-redux';
import store from '../stores'; // Adjust the import path to your store
import App from '../App'; // Adjust the import path to your main App component

// Mock the innerWidth property on the window object
function setWindowWidth(width) {
  Object.defineProperty(window, 'innerWidth', {
    writable: true,
    configurable: true,
    value: width,
  });
}

describe('MobileVirtualJoystick', () => {
  test('showJoystick is true when window width is less than 650px', () => {
    setWindowWidth(640); // Set the window width to less than 650px
    render(
      <Provider store={store}>
        <App />
      </Provider>
    );
    // Check if the state in the store is set correctly
    expect(store.getState().user.showJoystick).toBe(true);
  });

  test('showJoystick is false when window width is greater than or equal to 650px', () => {
    setWindowWidth(800); // Set the window width to greater than or equal to 650px
    render(
      <Provider store={store}>
        <App />
      </Provider>
    );
    // Check if the state in the store is set correctly
    expect(store.getState().user.showJoystick).toBe(false);
  });
});

Please note that you'll need to adjust the import paths to match your project's structure. Also, ensure that your Redux store is properly configured to allow for testing.

This test function checks the showJoystick state after rendering the main App component under different window widths. It's important to reset or clean up any mocked properties after your tests to avoid affecting other tests. You can use Jest's afterEach or afterAll hooks to do this.

[PR-Review]

Thank you for the opportunity to review your pull request for the virtual joystick feature. I've gone through the changes and provided specific feedback on several code changes. Overall, the feature seems to be well-integrated with the existing codebase, and the use of the react-joystick-component library is a good choice for this functionality.

Please ensure that all the feedback is addressed, especially regarding the initialization of new state properties, UI layout considerations, and potential conflicts with server-side rendering due to direct window object usage.

If you have any questions or need further clarification on any of the comments, feel free to reach out. Good luck with resolving the mentioned conflicts with PR kevinshen56714#62, and I look forward to seeing this feature in action!

[JWS-aAI]

@PR-Review Thanks! Real quick, what do you recommend i change first?

[PR-Review]

@JWS-aAI I recommend addressing the following items first, as they are critical for the functionality and stability of your feature:

1. Joystick State Initialization: Ensure that the joystickMovement property in MyPlayer.ts and PlayerSelector.ts is always initialized properly to avoid potential null reference issues when accessing its isMoving and direction properties.

2. Server-Side Rendering Compatibility: Modify the initialization of the showJoystick state in UserStore.ts to avoid direct usage of window.innerWidth, which can cause issues during server-side rendering or testing. Consider initializing this state in a component or through a middleware that is client-side specific.

3. UI Layout and Accessibility: Verify that the new UI elements, such as the joystick and buttons, do not interfere with other UI components and are easily accessible across different device sizes and resolutions.

Once these critical issues are addressed, you can proceed with the other feedback provided in the review comments to refine the feature and ensure a smooth user experience.