-
Notifications
You must be signed in to change notification settings - Fork 0
feat - UI 업데이트 #8
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR implements a comprehensive UI update for the frontend, introducing Korean Noto Serif fonts, redesigning the user interface with larger text sizes, replacing text inputs with wheel picker modals, and updating spacing and styling throughout the application.
- Integrated Noto Serif KR font family with multiple weights (Regular, SemiBold, Bold, ExtraBold)
- Redesigned UserInfoForm with custom wheel picker components replacing direct text inputs for birthdate, birth time, and notification time selection
- Updated typography sizes and spacing across LoginScreen, SettingsSheet, and CalendarPage
Reviewed changes
Copilot reviewed 9 out of 14 changed files in this pull request and generated 18 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/tailwind.config.js | Added Noto Serif KR font family configurations for Tailwind CSS |
| frontend/package.json | Added dependencies for Noto Serif KR fonts and expo-font |
| frontend/pnpm-lock.yaml | Updated lock file with new font dependencies |
| frontend/app.config.ts | Bumped app version from 1.0.0 to 1.0.1 |
| frontend/app/_layout.tsx | Integrated font loading logic with useFonts hook |
| frontend/app/index.tsx | Updated login flow logic, removed onSignUpSuccess callback, and added useEffect for sign-out state cleanup |
| frontend/src/components/UserInfoForm.tsx | Major refactoring: replaced text inputs with wheel picker modals, added date/time parsing utilities, and updated styling |
| frontend/src/components/SettingsSheet.tsx | Updated spacing, added safe area insets, and fixed duplicate background color classes |
| frontend/src/components/LoginScreen.tsx | Updated text sizes, spacing, placeholder text, and removed unused onSignUpSuccess prop usage |
| frontend/src/components/CalendarPage.tsx | Applied Noto font classes alongside existing font-serif classes and added quotation marks to fortune overview |
Files not reviewed (1)
- frontend/pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (1)
frontend/src/components/SettingsSheet.tsx:13
- Unused import Switch.
import {
ActivityIndicator,
Alert,
KeyboardAvoidingView,
Modal,
Platform,
Pressable,
Switch,
Text,
TextInput,
View,
} from 'react-native';
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <View className="p-4 "> | ||
| <View className="w-full max-w-xl gap-6 rounded-2xl bg-white p-5 "> |
Copilot
AI
Dec 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a trailing space in the className. This should be removed for cleaner code.
| <View className="p-4 "> | |
| <View className="w-full max-w-xl gap-6 rounded-2xl bg-white p-5 "> | |
| <View className="p-4"> | |
| <View className="w-full max-w-xl gap-6 rounded-2xl bg-white p-5"> |
| {month} | ||
| </Text> | ||
| <Text className="font-serif mb-1.5 ml-1.5 text-base text-gray-500">월</Text> | ||
| <Text className="font-serif mb-1.5 ml-1.5 text-base text-gray-500 font-noto"> |
Copilot
AI
Dec 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using both font-serif and font-noto classes on the same element is redundant. Only one font-family declaration should be used. Consider removing font-serif since font-noto is the intended font.
| <View className="items-end"> | ||
| <Text className="font-serif text-3xl font-bold" style={{ color: accentColor }}> | ||
| <Text | ||
| className="font-serif text-3xl font-bold font-noto" |
Copilot
AI
Dec 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using both font-serif and font-noto classes on the same element is redundant. Only one font-family declaration should be used. Consider removing font-serif since font-noto is the intended font.
| value={username} | ||
| onChangeText={setUsername} | ||
| placeholder="아이디" | ||
| placeholder="[email protected]" |
Copilot
AI
Dec 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a typo in the placeholder text. "exmaple" should be "example".
| placeholder="exmaple@mail.com" | |
| placeholder="example@mail.com" |
| 내 정보 | ||
| </Text> | ||
| <View className="overflow-hidden rounded-xl border border-gray-200 bg-white"> | ||
| <View className="overflow-hidden rounded-xl border border-gray-200 bg-white "> |
Copilot
AI
Dec 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's an extra trailing space in the className after "bg-white". This is unnecessary and should be removed for cleaner code.
| <View className="overflow-hidden rounded-xl border border-gray-200 bg-white "> | |
| <View className="overflow-hidden rounded-xl border border-gray-200 bg-white"> |
| <Text className="font-serif text-lg font-bold text-gray-900 font-noto-semibold">{label}</Text> | ||
| </View> | ||
| <Text className="font-serif text-[15px] leading-6 text-gray-600">{value}</Text> | ||
| <Text className="font-serif text-[15px] leading-6 text-gray-600 font-noto">{value}</Text> |
Copilot
AI
Dec 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using both font-serif and font-noto classes on the same element is redundant. Only one font-family declaration should be used. Consider removing font-serif since font-noto is the intended font.
| className="items-center justify-center" | ||
| > | ||
| <Text | ||
| className={`${isSelected ? 'text-gray-900' : 'text-gray-400'} ${itemTextClassName} `} |
Copilot
AI
Dec 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a trailing space in the className after "text-gray-400". Consider removing it for consistency.
| className={`${isSelected ? 'text-gray-900' : 'text-gray-400'} ${itemTextClassName} `} | |
| className={`${isSelected ? 'text-gray-900' : 'text-gray-400'} ${itemTextClassName}`} |
| <View className="p-4 "> | ||
| <View className="w-full max-w-xl gap-6 rounded-2xl bg-white p-5 "> |
Copilot
AI
Dec 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a trailing space in the className. This should be removed for cleaner code.
| <View className="p-4 "> | |
| <View className="w-full max-w-xl gap-6 rounded-2xl bg-white p-5 "> | |
| <View className="p-4"> | |
| <View className="w-full max-w-xl gap-6 rounded-2xl bg-white p-5"> |
| <View className="flex-row items-start justify-between"> | ||
| <View> | ||
| <Text className="font-serif text-xs tracking-[0.25em] text-gray-400">{year}</Text> | ||
| <Text className="font-serif text-xs tracking-[0.25em] text-gray-400 font-noto"> |
Copilot
AI
Dec 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using both font-serif and font-noto classes on the same element is redundant and confusing. Since both classes set the font-family property, only the last one in the CSS cascade will take effect. Consider using only the font-noto class (which applies the NotoSerifKR font) and removing font-serif, or verify which font family should actually be used.
| ) : fortune ? ( | ||
| <Text className="text-center font-serif text-lg font-semibold leading-7 text-gray-600"> | ||
| {fortune.overview} | ||
| <Text className="text-center font-serif text-lg font-semibold leading-7 text-gray-600 font-noto"> |
Copilot
AI
Dec 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using both font-serif and font-noto classes on the same element is redundant. Only one font-family declaration should be used. Consider removing font-serif since font-noto is the intended font.
작업내용