[FEATURE] Display most popular icons in Search for Icons page #8421

[ZeeMonk-pixel]

Fixes Issue

Related to #8421

Changes proposed

Show the 20 most popular icons on the page before search.

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

Note to reviewers

[eddiejaoude]

Thank you, please fix failing test on the GitHub action

[ZeeMonk-pixel]

Thank you, please fix failing test on the GitHub action

I’ll look further into it and fix.

[ZeeMonk-pixel]

Thank you, please fix failing test on the GitHub action

I’ll look further into it and fix.

@eddiejaoude I'll need help figuring out this failed test. I can't seem to wrap my head around it

[ckelwin]

@ZeeMonk-pixel As stated here, the test is failing because the expected result count does not match.

The test is doing:

• expect to have 20 (popular) icons listed by default - OK.
• search for icons using the term "mobile"
• expect to have at least 20 results returned (test is reusing the defaultIcons value) - KO. The original test was checking for "at least 5 results returned" which was OK.

    Expected: >= 20
    Received:    7

• Your PR added extra 'popular icons' to be displayed by default but should not affect the search result of the icons.

Should be fine to update the tests accordingly, to expect at least 7 results (see screenshot below).

https://github.com/ZeeMonk-pixel/BioDrop/blob/main/tests/icon.spec.js#L23

  await expect(results).toBeGreaterThanOrEqual(7);

[ZeeMonk-pixel]

@ZeeMonk-pixel As stated here, the test is failing because the expected result count does not match.

The test is doing:

• expect to have 20 (popular) icons listed by default - OK.
• search for icons using the term "mobile"
• expect to have at least 20 results returned (test is reusing the defaultIcons value) - KO. The original test was checking for "at least 5 results returned" which was OK.

    Expected: >= 20
    Received:    7

• Your PR added extra 'popular icons' to be displayed by default but should not affect the search result of the icons.

Should be fine to update the tests accordingly, to expect at least 7 results (see screenshot below).

https://github.com/ZeeMonk-pixel/BioDrop/blob/main/tests/icon.spec.js#L23

  await expect(results).toBeGreaterThanOrEqual(7);

Thank you for more insight to this, I'll look into it and complete this PR

[eddiejaoude]

Thank you for your contribution. But this does not solve the issue.

We can merge this, but we will leave the issue open and unassign it, because the issue is for making this list of icons dynamic out of the database and moving away from having it hardcoded.

[ZeeMonk-pixel]

Thank you for your contribution. But this does not solve the issue.

We can merge this, but we will leave the issue open and unassign it, because the issue is for making this list of icons dynamic out of the database and moving away from having it hardcoded.

Okay... so it's not just getting the popular ones from the database now but having the popular ones change as it changes on the database overtime?

[eddiejaoude]

Yes sorry if that was not clear, we are improving our issue descriptions

[ZeeMonk-pixel]

Yes sorry if that was not clear, we are improving our issue descriptions

okay. so if I'm getting this right... that means there'll be a getServerSideProps running the query to the database on every visit to the page?