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.

Sign up for GitHub

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

Jump to bottom

Add the IntroText and mobile homepage search bar #539

Merged
merged 8 commits into from
Feb 8, 2023
Merged

Add the IntroText and mobile homepage search bar #539

merged 8 commits into from
Feb 8, 2023

Conversation

AbdulrhmnGhanem
Copy link
Member

@AbdulrhmnGhanem AbdulrhmnGhanem commented Jan 3, 2023 •
edited
Loading

  • Refactor: SearchContext.jsx -> SearchContext.tsx
  • UI: improve mobile_menu breakpoint
  • UI: make the search bar usable in homepage too
  • UI: add search bar in home page for small devices
  • e2e: Homepage search on mobile

Fixes #511.

@AbdulrhmnGhanem AbdulrhmnGhanem marked this pull request as ready for review January 11, 2023 06:06
@kasbah
Copy link
Member

kasbah commented Jan 11, 2023

Some currently open PR is causing the + in the "add a project" button to be grey:

image

I'm not sure if it's this one or another one, but this one is touching the bar styles and is using grey...

@kasbah
Copy link
Member

kasbah commented Jan 11, 2023

I think the search input should remain in the results page on mobile as well.

@AbdulrhmnGhanem
Copy link
Member Author

I think the search input should remain in the results page on mobile as well.

I am not sure what you mean.

@kasbah
Copy link
Member

kasbah commented Jan 11, 2023 •
edited
Loading

I feel the experience is currently a little confusing on mobile. You search and then the search input seems to disappear. It goes into the hamburger menu but as a user you don't know that.

@AbdulrhmnGhanem
Copy link
Member Author

AbdulrhmnGhanem commented Jan 11, 2023 •
edited
Loading

The search input is visible in /search too. Could you add a screenshot of the page?

image

@kasbah
Copy link
Member

kasbah commented Jan 11, 2023

Ah, something weird is going on when there are no results.

Screenshot_20230111-234314

@kasbah
Copy link
Member

kasbah commented Jan 11, 2023

Also the "Kitspace is..." blurb isn't going away like it does on v1.

@AbdulrhmnGhanem
Copy link
Member Author

The copy and search field are visible now even if no search results exist.

image

I handled icon color regression too.

regression current
image image

@kasbah
Copy link
Member

kasbah commented Jan 12, 2023 via email

@AbdulrhmnGhanem AbdulrhmnGhanem force-pushed the ag/search-again branch 2 times, most recently from 8164ee7 to acd00ac Compare January 13, 2023 03:18
@kasbah
Copy link
Member

kasbah commented Jan 13, 2023

Now there's not enough of a gap between the search and results on mobile.

image

kasbah
kasbah requested changes Jan 17, 2023
View reviewed changes
Copy link
Member

@kasbah kasbah 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 a definite improvement so willing to merge as is with minor corrections. I think there are still some usability issues with mobile search but will open new issues for them.

frontend/src/components/ProjectCardGrid/index.tsx Show resolved Hide resolved
frontend/src/pages/search/index.tsx Outdated Show resolved Hide resolved
frontend/test/Copy.test.tsx Outdated Show resolved Hide resolved
frontend/test/fixtures/Copy.json Outdated Show resolved Hide resolved
@kasbah
Copy link
Member

kasbah commented Jan 17, 2023

Oh, and better PR title please.

@AbdulrhmnGhanem AbdulrhmnGhanem mentioned this pull request Jan 20, 2023
Open
@AbdulrhmnGhanem AbdulrhmnGhanem changed the title Mobile search Add the copy and mobile homepage search bar Jan 20, 2023
@kasbah
Copy link
Member

kasbah commented Jan 20, 2023 •
edited
Loading

Can we disambiguate "Copy" and call it "IntroText" or something?

@AbdulrhmnGhanem AbdulrhmnGhanem changed the title Add the copy and mobile homepage search bar Add the IntroText and mobile homepage search bar Jan 23, 2023
@AbdulrhmnGhanem
Copy link
Member Author

Can we disambiguate "Copy" and call it "IntroText" or something?

Changed the title, and component name in f5155ed.

@kasbah kasbah merged commit 4e56e65 into master Feb 8, 2023
@AbdulrhmnGhanem AbdulrhmnGhanem deleted the ag/search-again branch July 22, 2023 05:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kasbah kasbah kasbah requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Search bar should be front and center on home
2 participants
@AbdulrhmnGhanem @kasbah