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

Fix #1376: Showing BLANK with search terms return NO result #1413

Closed
wants to merge 1 commit into from

Conversation

NathanPang001
Copy link
Contributor

@NathanPang001 NathanPang001 commented Nov 19, 2020

Issue This PR Addresses

Fixes #1376

Type of Change

  • Bugfix: Change which fixes an issue
  • New Feature: Change which adds functionality
  • Documentation Update: Change which improves documentation
  • UI: Change which improves UI

Description

Solves #1376
Adding a no results image for when the user types something that doesn't match any result.

image

Checklist

  • Quality: This PR builds and passes our npm test and works locally
  • Tests: This PR includes thorough tests or an explanation of why it does not
  • Screenshots: This PR includes screenshots or GIFs of the changes made or an explanation of why it does not (if applicable)
  • Documentation: This PR includes updated/added documentation to user exposed functionality or configuration variables are added/changed or an explanation of why it does not(if applicable)

@NathanPang001
Copy link
Contributor Author

Huh, apparently its getting removed by "LOAD MORE POSTS" on the preview, I'm not sure why.

@NathanPang001
Copy link
Contributor Author

NathanPang001 commented Nov 19, 2020

Ah, I think it has to do with the LoadAutoScroll.jsx, hmm, I'm not sure how I should fix this.

@NathanPang001
Copy link
Contributor Author

I'll continue to fix this, if anyone check this, please tell me if you find any bugs or anything else.

Copy link
Contributor

@humphd humphd left a comment

Choose a reason for hiding this comment

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

We can't use a copyright image from Facebook, please remove this.

@yuanLeeMidori
Copy link
Contributor

yuanLeeMidori commented Jan 17, 2021

@NathanPang001 Hi! Telescope team is working on merging all open PRs. Are you still working on this PR? We're happy to assist you to keep working on this PR. However, please let us know if you don't have interest keeping working on it. We'll assign another team member to work on it and make it merged. Thank you :)

@yuanLeeMidori yuanLeeMidori changed the title Issue #1376: Showing BLANK with search terms return NO result Fix #1376: Showing BLANK with search terms return NO result Jan 19, 2021
@humphd humphd added this to the 1.6 Release milestone Jan 19, 2021
@izhuravlev
Copy link
Contributor

We should take a look around for another royalty-free image that might fit our project and will not violate any copyrights.
Possible sources: https://unsplash.com/ https://undraw.co/

@izhuravlev izhuravlev self-assigned this Jan 22, 2021
@izhuravlev
Copy link
Contributor

Can this be considered "Success"?
image

@izhuravlev
Copy link
Contributor

izhuravlev commented Feb 5, 2021

Possible replacement for the "No Results Found" picture:

  1. Mistaken Scientist
  2. I am sorry
  3. Doggo
  4. Empty
  5. Magnifying Glass
  6. Searching from UnDraw:
    image

@humphd @chrispinkney @birtony and everybody else are welcome to review them and select your personal favourite

@humphd
Copy link
Contributor

humphd commented Feb 5, 2021

I like where this is heading. I hadn't thought about using photos, but that would be something we could do more of, since we're doing the image service in the back-end.

I wonder if photos vs. illustrations are best for our overall aesthetic, since we use that on the home page for the hero banner?

@birtony
Copy link
Contributor

birtony commented Feb 5, 2021

From my perception, I think the illustration fits better for the no results @humphd.

I like where this is heading. I hadn't thought about using photos, but that would be something we could do more of, since we're doing the image service in the back-end.

I wonder if photos vs. illustrations are best for our overall aesthetic, since we use that on the home page for the hero banner?

@birtony
Copy link
Contributor

birtony commented Feb 5, 2021

Possible replacement for the "No Results Found" picture:

1. [Mistaken Scientist](https://unsplash.com/photos/8RXmc8pLX_I)

2. [I am sorry](https://unsplash.com/photos/S4tm-mox-xQ)

3. [Doggo](https://unsplash.com/photos/K4mSJ7kc0As)

4. [Empty](https://unsplash.com/photos/-x-Brii2QaM)

5. [Magnifying Glass](https://unsplash.com/photos/uAFjFsMS3YY)

6. Searching from UnDraw:
   ![image](https://user-images.githubusercontent.com/36719387/107037172-b718c880-6788-11eb-9065-750116b93624.png)

@humphd @chrispinkney @birtony and everybody else are welcome to review them and select your personal favourite

Number 6 is my favourite out of these

@humphd
Copy link
Contributor

humphd commented Feb 5, 2021

OK, let's use:

image

Is it licensed for reuse? If so, let's do it now for 1.6 and iterate. I want this in today for 1.6

@@ -7,6 +7,7 @@ import { Container } from '@material-ui/core';
import useSiteMetadata from '../../hooks/use-site-metadata';
import Timeline from '../Posts/Timeline.jsx';
import Spinner from '../Spinner';
import NotFound from '../../images/NotFound.jpg';

Copy link
Contributor

Choose a reason for hiding this comment

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

Should this image be put in the public folder?

@@ -61,7 +66,9 @@ const SearchResults = ({ text, filter }) => {
{data && data.length ? (
<Timeline pages={data} nextPage={() => setSize(size + 1)} />
) : (
<h1>No search results</h1>
<h1 className={classes.noResults}>
<img src={NotFound} alt="Not Found" />
Copy link
Contributor

Choose a reason for hiding this comment

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

would a div tag more appropriate here? h1 tends to be the title of the page.

Copy link
Contributor

Choose a reason for hiding this comment

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

Agree, I would <div> here as well

@@ -7,6 +7,7 @@ import { Container } from '@material-ui/core';
import useSiteMetadata from '../../hooks/use-site-metadata';
import Timeline from '../Posts/Timeline.jsx';
import Spinner from '../Spinner';
import NotFound from '../../images/NotFound.jpg';
Copy link
Contributor

Choose a reason for hiding this comment

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

We should make sure to run this image (or whichever one we end up using) through https://tinyjpg.com/ or https://tinypng.com/. For the current one, it would reduce its size by 53%:
image
This would positively affect our site loading speed.

@@ -61,7 +66,9 @@ const SearchResults = ({ text, filter }) => {
{data && data.length ? (
<Timeline pages={data} nextPage={() => setSize(size + 1)} />
) : (
<h1>No search results</h1>
<h1 className={classes.noResults}>
<img src={NotFound} alt="Not Found" />
Copy link
Contributor

Choose a reason for hiding this comment

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

Agree, I would <div> here as well

@izhuravlev
Copy link
Contributor

@humphd I don't think we will be able to use it - it is their image, and I think most of what I can do with it is screenshot it - I can't find it on their platform.

@izhuravlev
Copy link
Contributor

Closed by #1700. Thank you @NathanPang001 for your work!

@izhuravlev izhuravlev closed this Feb 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: nextjs Nextjs related issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Showing BLANK with search terms return NO result
7 participants