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

[EAM-4101] Refine Search Page #221

Open
wants to merge 36 commits into
base: master
Choose a base branch
from
Open

[EAM-4101] Refine Search Page #221

wants to merge 36 commits into from

Conversation

karolkos-glitch
Copy link

@karolkos-glitch karolkos-glitch commented Jan 15, 2025

  • convert class components in the function components
  • adjust components tree structure - create new smaller children components in SearchHeader, use render props pattern SearchResults
  • refine data flow - use render props pattern to avoid props drilling, delete some useless state (isPhoneView) or some props
  • add comments for somethings that I think are not clear to me as a new developer in this codebase
  • propose some solutions to refine Search Page

@karolkos-glitch karolkos-glitch self-assigned this Jan 15, 2025
Copy link
Author

@karolkos-glitch karolkos-glitch left a comment

Choose a reason for hiding this comment

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

I'm aware some things are still kind of draft here, however I would like get some feedback if I can proceed with the changes or I should drop the caching results logic and clean up and finish 100% the convertion of the code to functional components

src/ui/pages/search/useSearchResources.js Show resolved Hide resolved
src/ui/pages/search/SearchHeaderInput.jsx Show resolved Hide resolved
.gitignore Outdated Show resolved Hide resolved
.nvmrc Outdated Show resolved Hide resolved
@karolkos-glitch
Copy link
Author

Screenshot 2025-01-20 at 08 11 23

When I'm running npm run lint I'm getting 1283 problems. I understand the previous changes in the codebase (migration from cra to vite) could bring a lot of new things here, however it might be good actually to solve this issue, install something like prettier and setup the CI to ensure all PRs are written in the same way and the code formatting in the whole codebase is also the same?

@karolkos-glitch karolkos-glitch marked this pull request as ready for review January 20, 2025 07:15
@karolkos-glitch karolkos-glitch force-pushed the EAM-4101 branch 3 times, most recently from 67bb76f to 4324d02 Compare January 31, 2025 08:31
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.

4 participants