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

M1-TOP Page / UI #9

Merged
merged 7 commits into from
Dec 2, 2023
Merged

M1-TOP Page / UI #9

merged 7 commits into from
Dec 2, 2023

Conversation

kanatagu
Copy link
Contributor

@kanatagu kanatagu commented Nov 27, 2023

Description

  • Add Top Page UI
  • API data, GraphQL will be next PR after I pull the GraphQL setup code

URL is http://localhost:3000/

Fixes # (issue)

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Screenshot

PC
screencapture-localhost-3000-2023-11-28-00_04_01
screencapture-localhost-3000-2023-11-28-00_04_10

Tablet

SP

@@ -0,0 +1 @@
20.8.0
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought we need it so I added

Copy link
Contributor

@Yo-mah-Ya Yo-mah-Ya Nov 29, 2023

Choose a reason for hiding this comment

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

Nice, but different version ?

node-version: '18.16.0'

I'd say both have to be same version.

Probably 20.8.0 would be better, due to Next.js version 14's node version prerequisites.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Yo-mah-Ya
Thank you! I changed it
f7e13af

import { InputSearch } from '@/components/input'

export const TripSearch = () => {
return <InputSearch />
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 component has meaning. I will add more to this component in next PR

app/page.tsx Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

I noticed that when the screen size is md (768px - 991px), the card component takes up 100% of the width, and it looks too big. It might look better if it were set to 70% width or something like that?
Screenshot 2023-11-28 at 10 30 18 AM

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh I did it on purpose! I thought it's better to make the width as 100% since the other components are width 100% and I wanted to arrange it in same line. I could do two columns for iPad instead but I just did one column as SP size. Maybe it's better?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@SachiGoto
I changed to two columns! M1-Top_ui

screencapture-localhost-3000-2023-11-28-15_08_28screencapture-localhost-3000-2023-11-28-15_08_44

Copy link
Contributor

@SachiGoto SachiGoto left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Contributor

@samuraikun samuraikun left a comment

Choose a reason for hiding this comment

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

LGTM

@kanatagu kanatagu merged commit 0ce8fa4 into main Dec 2, 2023
1 check passed
@kanatagu kanatagu deleted the M1-Top_ui branch December 2, 2023 00:53
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.

4 participants