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

Replace new listing search with an autocomplete #144

Merged
merged 7 commits into from
Jul 12, 2024

Conversation

RyotaMitaraiWeb
Copy link
Collaborator

Closes #141

The new listing page now has an autocomplete. Once the user types in something, a debounced request (debounced by a half a second) will make a search request and populate the menu options below it. Clicking any of those will redirect you to the create page for the respective card

Notes:

  • although the options are rendered as hyperlinks, there is still a function that triggers when an option is selected that performs a programmatic navigation. The reason for that is to support keyboard navigation as well (as you can cycle through the options with arrow keys and the Home and End keys, as well as select an option with Enter), in which case hyperlinks aren't enough. This also has the benefit of things like URL preview (at the bottom of most browsers) and ability to open the page in a new tab.
  • Because the search endpoint is paginated, the list will never display more than ten items. This is something that should probably be fixed before we release a stable version.
  • Cards with particularly long names will require horizontal scrolling. The only (reasonably achievable) way to fix this is to make the search field wider.

@RyotaMitaraiWeb RyotaMitaraiWeb added this to the v1.4 milestone Jul 8, 2024
@RyotaMitaraiWeb RyotaMitaraiWeb added frontend bug Something isn't working labels Jul 8, 2024
@julkascript
Copy link
Owner

julkascript commented Jul 10, 2024

Nice, looks good overall!

Two things, though:

  1. I get a "key" error in the console when react tries to render the items. Maybe we need to take a look or is it just me?
  2. (If not too big of a hustle) Please put the magnifying glass icon in the beginning (so it can look like the search bar in the navigation).

@RyotaMitaraiWeb
Copy link
Collaborator Author

I couldn't replicate the issue with keys, I would need more details to reproduce it

@julkascript
Copy link
Owner

I recorded a gif for you:

err

Also, here is the error log:

Warning: A props object containing a "key" prop is being spread into JSX:
  let props = {key: someKey, tabIndex: ..., role: ..., id: ..., onMouseMove: ..., onClick: ..., onTouchStart: ..., data-option-index: ..., aria-disabled: ..., aria-selected: ..., className: ..., children: ...};
  <ForwardRef(MenuItem5) {...props} />
React keys must be passed directly to JSX without using spread:
  let props = {tabIndex: ..., role: ..., id: ..., onMouseMove: ..., onClick: ..., onTouchStart: ..., data-option-index: ..., aria-disabled: ..., aria-selected: ..., className: ..., children: ...};
  <ForwardRef(MenuItem5) key={someKey} {...props} />
    at Autocomplete2 (

@RyotaMitaraiWeb
Copy link
Collaborator Author

RyotaMitaraiWeb commented Jul 11, 2024

So I didn't manage to reproduce the error in the console, despite also trying it on Brave myself. However, I think I figured out what's going on.

From what I am reading, the props value passed to the render prop actually has a property known as key (which I subsequently confirmed to be true). When I was spreading the props directly onto the MenuItem component, it passed its own key to it and that presumably caused React to complain. I have committed a change to not pass it at all, but I need a confirmation that it solves the problem because, again, I never managed to reproduce the error in my own browsers (despite doing the same things as in the GIF).

@julkascript
Copy link
Owner

Yep, correct, well done! LGTM!

@julkascript julkascript merged commit 59eb025 into develop Jul 12, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working frontend
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Creating a listing does not allow you to see all sets
2 participants