Skip to content

[UX/UI] Some fixes in search autocomplete #2771

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

Merged
merged 4 commits into from
Jun 10, 2023
Merged

Conversation

arielj
Copy link
Collaborator

@arielj arielj commented Jun 3, 2023

This PR includes a fix for #2575 and some other fixes I noticed while fixing that:

  • if we had the same game in 2 stores, there was no way to select which one to open and Epic had priority (this was not an issue when clicking only changed the results), I added a (GOG) or (Epic) or (sideload) suffix
  • we were ordering and then filtering, which is slower than first filtering and then ordering since then we need to order less elements
  • we were converting the list of games to a list of titles to render it and then on click we were fetching the game again, now it just uses the list of games to render it and passes the game as an argument on click to avoid that extra work
  • since now we only have unique games, no need to convert everything into a set and then back to an array, so now it's all a single array
  • it now ignores DLCs from the search result, which created issues (before it was producing the error in the issue linked, after the element changed to open the game page it was opening the DLC page which doesn't really work)

image


Use the following Checklist if you have changed something on the Backend or Frontend:

  • Tested the feature and it's working on a current and clean install.
  • Tested the main App features and they are still working on a current and clean install. (Login, Install, Play, Uninstall, Move games, etc.)
  • Created / Updated Tests (If necessary)
  • Created / Updated documentation (If necessary)

@arielj arielj added the pr:ready-for-review Feature-complete, ready for the grind! :P label Jun 3, 2023
@arielj arielj requested review from a team, flavioislima, CommandMC, Nocccer and imLinguin and removed request for a team June 3, 2023 17:47
@flavioislima
Copy link
Member

Can you add a small fix, probably one line, to the shortcuts creator? I think Dlcs after being installed are cresting shortcuts and being added to steam and desktop if these options are on on the settings.

So just need to check if it's a dlc before adding the shortcuts. 👍

@arielj
Copy link
Collaborator Author

arielj commented Jun 3, 2023

Can you add a small fix, probably one line, to the shortcuts creator? I think Dlcs after being installed are cresting shortcuts and being added to steam and desktop if these options are on on the settings.

So just need to check if it's a dlc before adding the shortcuts. +1

I'll do that in a different PR, there's also an error related to shortcuts for sideloaded apps and that change will make more sense there #2756

Copy link
Member

@flavioislima flavioislima left a comment

Choose a reason for hiding this comment

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

LGTM

@arielj
Copy link
Collaborator Author

arielj commented Jun 10, 2023

hey @flavioislima I can't merge this, I think it's because I skipped CI in the last commit (because it was just removing a comment), maybe you can merge since you are the owner

@flavioislima flavioislima merged commit 1e313e5 into main Jun 10, 2023
@flavioislima flavioislima deleted the search-autocomplete-fixes branch June 10, 2023 14:29
@flavioislima
Copy link
Member

hey @flavioislima I can't merge this, I think it's because I skipped CI in the last commit (because it was just removing a comment), maybe you can merge since you are the owner

Done

@Nocccer
Copy link
Collaborator

Nocccer commented Jun 10, 2023

hey @flavioislima I can't merge this, I think it's because I skipped CI in the last commit (because it was just removing a comment), maybe you can merge since you are the owner

FYI: You can retrigger checks inside the pr unser Checks or you use the vscode GitHub Action extension

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr:ready-for-review Feature-complete, ready for the grind! :P
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants