-
Notifications
You must be signed in to change notification settings - Fork 0
홈 화면에 TopAppBar를 추가합니다. #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 adds a TopAppBar component to the design system and integrates it into the home screen. The changes introduce a new reusable TopAppBar component and updates the home screen layout to use this component with a my page button.
- Adds a new TukTopAppBar component to the design system
- Integrates the TopAppBar into the home screen with a my page action button
- Replaces the old user icon with a new my page icon
Reviewed Changes
Copilot reviewed 6 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| TukApp.kt | Updates Scaffold with white background color |
| HomeScreen.kt | Restructures layout to include TopAppBar and adds preview function |
| ic_user.xml | Removes old user icon drawable |
| ic_mypage.xml | Adds new my page icon drawable |
| TopAppBar.kt | Creates new TukTopAppBar component |
| build.gradle.kts | Adds Compose library dependency to design system module |
| Scaffold( | ||
| modifier = modifier.fillMaxSize() | ||
| modifier = modifier.fillMaxSize(), | ||
| containerColor = Color.White |
Copilot
AI
Jul 23, 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 hardcoded Color.White may not support theming or dark mode. Consider using a theme-aware color from MaterialTheme.colorScheme.background or defining it in your design system.
| containerColor = Color.White | |
| containerColor = MaterialTheme.colorScheme.background |
| modifier = modifier | ||
| .height(64.dp) | ||
| .fillMaxWidth() | ||
| .padding(end = if(actionButtons == null) 20.dp else 12.dp), |
Copilot
AI
Jul 23, 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.
Missing space after 'if' keyword. Should be 'if (actionButtons == null)' according to Kotlin coding conventions.
| .padding(end = if(actionButtons == null) 20.dp else 12.dp), | |
| .padding(end = if (actionButtons == null) 20.dp else 12.dp), |
| modifier = modifier | ||
| .height(64.dp) | ||
| .fillMaxWidth() | ||
| .padding(end = if(actionButtons == null) 20.dp else 12.dp), |
Copilot
AI
Jul 23, 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.
Magic numbers 20.dp and 12.dp should be extracted as named constants or defined in a dimensions resource file for better maintainability.
| .padding(end = if(actionButtons == null) 20.dp else 12.dp), | |
| .padding(end = if(actionButtons == null) DEFAULT_PADDING else ACTION_BUTTONS_PADDING), |
작업