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

BB-726: refactor drag-and-drop component #966

Merged
merged 10 commits into from
Oct 10, 2023
Merged

Conversation

meziyum
Copy link
Contributor

@meziyum meziyum commented Mar 14, 2023

Problem BB-726

The React components used on the bookbrainz are of both class and functional types. In React, both class components and functional components are used to create UI elements. However, with the introduction of React Hooks, functional components have become much more powerful and flexible, and they can now do everything that class components can do.

Functional components are generally preferred over class components for several reasons:

  1. Simplicity and readability
  2. Performance
  3. Easier to test
  4. Better compatibility with hooks:

Solution

The refactored code replaces the class component with a function component. Specifically, it uses the useState and useCallback hooks to manage state and event handlers respectively. The handleClick, handleDragOver, and handleDrop methods in the class component are now replaced with callback functions that are defined using the useCallback hook.

The refactored code also removes the constructor method and instead initializes the state using the useState hook. The addChild method is now replaced with a new function defined within the component scope, and the state is updated using the setAchievement function returned by the useState hook.

Overall, the refactored code simplifies the class component by using React hooks, reducing the amount of boilerplate code, and making the code easier to read and understand.

Areas of Impact

Achievement Pages

Copy link
Member

@MonkeyDo MonkeyDo left a comment

Choose a reason for hiding this comment

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

Thank you for this improvement !

While we are here refactoring, could you please make the file type safe with typescript?
You will need to change the file extension to .tsx and make the code comply with TypeScript types. I'm happy to help if you are unfamiliar with TS.

src/client/components/input/drag-and-drop.js Outdated Show resolved Hide resolved
@meziyum
Copy link
Contributor Author

meziyum commented Mar 21, 2023

Thank you for this improvement !

While we are here refactoring, could you please make the file type safe with typescript? You will need to change the file extension to .tsx and make the code comply with TypeScript types. I'm happy to help if you are unfamiliar with TS.

Well yes I have yet to gain proficiency in Typescript so will need a little guidance.

Q1. How should we handle the error for parsing or should we not handle it at all?

Q2. Which would be better suited for this type or interface?

Q3. Should we use both the type of Typescripts as well as propTypes or not?

Also while we are at it. Should I go back and do the same for the achievement component in #950 . I did refactor it but didnt make the types safe.

meziyum and others added 4 commits March 22, 2023 03:22
The .js file has been converted to .tsx for proper typesafe.

The tsconfig has strict mode set to false even then we are using event definition in typescript for future proofing in case we need to change the mode to strict.
@meziyum meziyum requested a review from MonkeyDo March 22, 2023 03:01
@MonkeyDo MonkeyDo merged commit ee90135 into metabrainz:master Oct 10, 2023
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.

2 participants