Skip to content
This repository has been archived by the owner on Aug 7, 2024. It is now read-only.

fix: refactor search page structure #2196

Merged
merged 2 commits into from
Nov 11, 2022
Merged

fix: refactor search page structure #2196

merged 2 commits into from
Nov 11, 2022

Conversation

schmelto
Copy link
Contributor

@schmelto schmelto commented Nov 10, 2022

Updated:

image

user not found under the search bar.

Old:
image

This change prevents that the search bar is jumping on the page when the error message appears

Further added a warning message if the search string is less than 3 characters
image

Remove the results list if the search string is less than 3 characters

@github-actions github-actions bot added small Pull request with less than 10 changed lines kudos labels Nov 10, 2022
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

It's great having you contribute to this project

Welcome to the community 🤓

If you would like to continue contributing to open source and would like to do it with an awesome inclusive community, you should join our Discord chat and our GitHub Organisation - we help and encourage each other to contribute to open source little and often 🤓 . Any questions let us know.

@github-actions
Copy link
Contributor

github-actions bot commented Nov 10, 2022

Reviewpad Report

ℹ️ Messages

  • Kudos for your 10th pull request! You are awesome

remove results if under 3 characters
@github-actions github-actions bot added the medium Pull request with changed lines between 10 and 30 label Nov 10, 2022
@schmelto schmelto removed the small Pull request with less than 10 changed lines label Nov 10, 2022
Comment on lines +83 to +87
{!threeOrMore && (
<h2 className="bg-yellow-200 text-yellow-600 border-2 border-yellow-600 p-5 my-5 text-xl">
You have to enter at least 3 characters to search for a user.
</h2>
)}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is something we should discuss if this is more suitable as the text in the search box

Copy link
Member

Choose a reason for hiding this comment

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

we could also move the alert to a separate component in another PR, maybe it accepts a type of alert (info, warning, error)

Copy link
Member

@eddiejaoude eddiejaoude left a comment

Choose a reason for hiding this comment

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

Looks great 👍 thanks Tom!

@eddiejaoude eddiejaoude merged commit d0439fb into EddieHubCommunity:main Nov 11, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
medium Pull request with changed lines between 10 and 30 waiting-for-reviewers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants