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

Hero and Searchbar #1

Merged
merged 4 commits into from
Apr 5, 2023
Merged

Hero and Searchbar #1

merged 4 commits into from
Apr 5, 2023

Conversation

meowyx
Copy link
Collaborator

@meowyx meowyx commented Apr 3, 2023

This is my progress on the hero and searchbar:

  • created components
  • used mock data on the searchbar
  • rendered the components on the page

</div>
</div>
</div>
{showResults && (
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to separate state showResults ? can't we just check the length of results ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@chin-flags I used showResults to show data on focus. But I guess I can see if I can do what you suggested.
I tried to map data on results so that it doesnt show more than 3 while rendering.

@@ -3,13 +3,14 @@ import Image from "next/image";
import { Inter } from "next/font/google";
import styles from "@/styles/Home.module.css";
import Text from "../../components/Text";
import Hero from "../../components/Hero";

const inter = Inter({ subsets: ["latin"] });

export default function Home() {
Copy link
Contributor

Choose a reason for hiding this comment

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

minor inconstancy;
here its a regular function and Hero is an arrow function

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@chin-flags Thank you for pointing out. I ended up using the starter code on this that is why the inconsistency.

@@ -0,0 +1,5 @@
export interface DummyData {
Copy link
Contributor

Choose a reason for hiding this comment

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

can there be overlaps ? examples:
can an item in trendingNow also be in editorsPicks ? and a given item in trendingNow falls under a certain category ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@chin-flags yes there can be overlaps on data but I currently don't know how the data will be. Is there any method you'd suggest?

Copy link
Contributor

Choose a reason for hiding this comment

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

make sense!. we can always update this interface once we have more clarity

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

totally!

@meowyx
Copy link
Collaborator Author

meowyx commented Apr 5, 2023

According to @chin-flags 's review. I have done the following:

  • removed showResults state and worked with results only.
  • updated the normal function into arrow function

@meowyx meowyx requested a review from chin-flags April 5, 2023 17:47
@meowyx meowyx merged commit 3acd42d into main Apr 5, 2023
@meowyx meowyx deleted the hero-searchbar branch July 17, 2023 14:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants