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.

Sign up for GitHub

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

Jump to bottom

Fix (9738): Prevent showing the initial state when changing query [search page] #10276

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

Fix (9738): Prevent showing the initial state when changing query [search page] #10276

wants to merge 4 commits into from

Conversation

stefanzmf
Copy link

Fixes Issue

Closes #9738

Changes proposed

  • Add reusable Loading component in the components folder (The Loading is aligned both, vertically and horizontally inside the parent)
  • Set a local state for handling the loading status (set it to true when the fetchUsers function is called and false when it ends)

Extra:

  • Remove duplicate logic for rendering the list of the users
  • Render one piece of content at the time by chaining all the ifs

Check List (Check all the applicable boxes)

  • My code follows the code style of this project.
  • My change requires changes to the documentation.
  • I have updated the documentation accordingly.
  • All new and existing tests passed.
  • This PR does not contain plagiarized content.
  • The title of my pull request is a short description of the requested changes.

Screenshots

Screen.Recording.2024-03-13.at.14.13.33.mov

Note to reviewers

Do you think I should include a fix for this warning as well?

./pages/search.js
153:6  Warning: React Hook useEffect has missing dependencies: 'BASE_URL', 'pathname', and 'replace'. Either include them or remove the dependency array.  react-hooks/exhaustive-deps

@stefanzmf
Add reusable Loading component:
1999bbe 
- add aria specific attributes
- use items-center and justify-center in combination with h-full in order to show it in the center of the container
- use hardcoded fa-spinner icon in combination with animate-spin class for a nicer effect
@stefanzmf
Add Loading spinner for search page:
d93c255 
- set isLoading flag and set it based on the fetchUsers status and contitional render content based on its value
@stefanzmf
Simplify logic for render visible users:
8a06b9c 
- check add extra logic complexity and code duplication
- the visibile users still contains all the users if the users length is smaller than usersPerPage
@stefanzmf
Extend conditional render to ensure only one component is shown at th…
96e077b 
…e time
@github-actions github-actions bot added the issue linked Pull Request has issue linked label Mar 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
issue linked Pull Request has issue linked
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[BUG] Search page rolls back to initial state whenever a new search filter is added/removed.
1 participant
@stefanzmf