[Chore] 16KB 페이지 지원을 위한 lib 버전 업그레이드 #131
Conversation
|
늦은 코드리뷰 죄송합니다! 전체적으로 확인해보니 이미지 권한 수정과 네이버 맵 버전 업데이트를 하신 것 같더라구요 |
다른 16KB 대응 체크리스트가 어떤걸 말하는건가요?? |
WalkthroughReplaced legacy image-picking with the Android Photo Picker API, introduced a build-time NAVER_MAP_ID manifest placeholder and bumped the Naver Maps library, and applied small layout/accessibility and button sizing adjustments. Changes
Sequence DiagramsequenceDiagram
participant User
participant Activity as CommunityPostWriteActivity
participant Picker as Android Photo Picker
participant Upload as Image Upload Service
User->>Activity: tap "select gallery"
Activity->>Picker: launch(PickVisualMediaRequest ImageOnly)
Note right of Picker #DDEBF7: system photo picker UI shown
User->>Picker: pick image or cancel
Picker-->>Activity: returns URI or null
alt URI returned
Activity->>Upload: upload(uri)
Upload-->>Activity: returns uploaded image URL
Activity->>Activity: insert image URL at cursor
else Cancelled / no selection
Activity-->>User: no change / handle gracefully
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (1 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/src/main/res/layout/fragment_community_post_list.xml (1)
155-156: Remove deprecatedsingleLineattribute.
android:singleLine="true"(line 155) is deprecated and functionally redundant withandroid:maxLines="1"(line 156). Keep onlymaxLines="1".- android:text="글쓰기" - android:elevation="4dp" - android:textColor="@color/white" - android:textSize="15sp" - android:textStyle="bold" - android:singleLine="true" - android:maxLines="1" + android:text="글쓰기" + android:elevation="4dp" + android:textColor="@color/white" + android:textSize="15sp" + android:textStyle="bold" + android:maxLines="1"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
app/build.gradle.kts(1 hunks)app/src/main/AndroidManifest.xml(2 hunks)app/src/main/java/com/kust/kustaurant/presentation/ui/community/CommunityPostWriteActivity.kt(5 hunks)app/src/main/res/layout/activity_community_post_detail.xml(5 hunks)app/src/main/res/layout/fragment_community_post_list.xml(2 hunks)gradle/libs.versions.toml(1 hunks)
🔇 Additional comments (8)
app/src/main/res/layout/fragment_community_post_list.xml (1)
139-139: Verify wrap_content width aligns with 16KB page support objective.The button width was changed from fixed
104dptowrap_content. With the existing end constraint to the guideline, the button will now size dynamically based on content. Please clarify whether this layout change is necessary for 16KB page size support or if it's a separate UI refinement.app/src/main/res/layout/activity_community_post_detail.xml (2)
371-371: Confirm textSize unit change supports 16KB page size compatibility.Line 371 shows a
textSizechange on the EditText. Ensure this was changed tosp(scale-independent pixels) rather thandp, assprespects user font size preferences and is the recommended unit for text sizing in Android. This aligns with the PR's goal of supporting 16KB page sizes on compatible devices.
391-393: Padding adjustments for confirm button are appropriate.The added
paddingStart="0dp"andpaddingEnd="3dp"on the confirm button's ImageView provide fine-tuned visual alignment. Ensure this padding adjustment doesn't cause visual misalignment or overflow on different screen densities.app/src/main/AndroidManifest.xml (1)
120-122: LGTM! Secure API key management.Moving the Naver Maps client ID to a build-time placeholder keeps sensitive credentials out of version control and aligns with Android security best practices.
app/build.gradle.kts (1)
44-44: LGTM! Manifest placeholder correctly configured.The NAVER_MAP_ID placeholder is properly configured and follows the same pattern as the existing KAKAO_NATIVE_KEY. Ensure that
naver_map_idis documented in your setup instructions or README for other developers.app/src/main/java/com/kust/kustaurant/presentation/ui/community/CommunityPostWriteActivity.kt (2)
16-16: Excellent migration to Photo Picker API.The migration from the legacy
startActivityForResultpattern to the modern Photo Picker API is the correct approach for Android 13+ and aligns with 16KB page support requirements. This provides better user privacy and requires fewer permissions.Also applies to: 38-38
163-183: Well-implemented Photo Picker integration.The implementation correctly uses
ActivityResultContracts.PickVisualMediawith:
- Proper registration in
initPhotoPicker()ImageOnlyfilter for appropriate media selection- Coroutine-based async upload handling
- Clean separation of concerns
This eliminates the need for manual permission handling on Android 13+ while maintaining backward compatibility through the
READ_EXTERNAL_STORAGEpermission (maxSdkVersion="32") in the manifest.gradle/libs.versions.toml (1)
24-24: I need to search more specifically for GitHub release information to verify the 16KB page support claim and check for any documented breaking changes.Let me search more specifically for 16KB page support and NDK-related compatibility:
Based on my verification, I can now provide the rewritten review comment.
Verify Naver Maps SDK compatibility and test thoroughly before publishing.
The upgrade from 3.17.0 to 3.23.0 spans six minor versions. Starting November 1st, 2025, all new apps and updates to existing apps submitted to Google Play and targeting Android 15+ devices must support 16 KB page sizes, and if your app uses any NDK libraries, either directly or indirectly through an SDK, then you will need to rebuild your app for it to work on these 16 KB devices. Naver Maps SDK contains native code (.so files), making this upgrade likely necessary for Google Play compliance.
Ensure that:
- The upgrade has been tested with your map features
- Any API changes or deprecations have been addressed
- The new version is stable in your target environments
| android:id="@+id/community_iv_user_icon" | ||
| android:layout_width="16dp" | ||
| android:layout_height="16dp" | ||
| android:contentDescription="유저 아이콘 입니다." |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Externalizing contentDescription strings to strings.xml for consistency and localization.
The PR adds contentDescription attributes to three ImageViews (lines 131, 282, 309) as accessibility improvements, which is good. However, the implementation is inconsistent: some ImageViews in this file reference string resources (e.g., line 45: @string/back_btn_info, line 151: @string/community_post_img_info), while the new ones use inline Korean strings.
For consistency and to support localization, move these inline contentDescription strings to strings.xml:
<!-- In strings.xml -->
<string name="community_user_icon_info">유저 아이콘 입니다.</string>
<string name="community_like_icon_info">좋아요 아이콘 입니다.</string>
<string name="community_scrap_icon_info">스크랩 아이콘입니다.</string>Then update the layout:
- android:contentDescription="유저 아이콘 입니다."
+ android:contentDescription="@string/community_user_icon_info"
- android:contentDescription="좋아요 아이콘 입니다."
+ android:contentDescription="@string/community_like_icon_info"
- android:contentDescription="스크랩 아이콘입니다."
+ android:contentDescription="@string/community_scrap_icon_info"Also applies to: 282-282, 309-309
🤖 Prompt for AI Agents
In app/src/main/res/layout/activity_community_post_detail.xml at lines 131, 282
and 309, three ImageView contentDescription attributes use inline Korean strings
which breaks consistency with other entries and prevents localization; move
those inline strings into app/src/main/res/values/strings.xml using resource
names like community_user_icon_info, community_like_icon_info and
community_scrap_icon_info (with the provided Korean values), then replace the
inline contentDescription values in the layout with references to
@string/community_user_icon_info, @string/community_like_icon_info and
@string/community_scrap_icon_info respectively so all contentDescription
attributes consistently use string resources and support localization.
|
Note Unit test generation is an Early Access feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
|
Caution The CodeRabbit agent failed during execution: Clone operation failed |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/src/main/res/layout/activity_community_post_detail.xml(5 hunks)app/src/main/res/layout/fragment_community_post_list.xml(2 hunks)app/src/main/res/values/strings.xml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/src/main/res/layout/activity_community_post_detail.xml
🔇 Additional comments (2)
app/src/main/res/layout/fragment_community_post_list.xml (1)
137-157: Button sizing improvements support responsive layout.The change from fixed width (104dp) to
wrap_contentincreases layout flexibility across screen densities and text variations (including i18n). ThemaxLines="1"constraint ensures text stays single-line, maintaining the compact button design. With horizontal padding (20dp start/end) and a drawable end, the button will size appropriately based on content while respecting the fixed 37dp height and visual constraints.app/src/main/res/values/strings.xml (1)
34-36: Accessibility strings added for community icon descriptions.The three new content-description strings (
community_user_icon_info,community_like_icon_info,community_scrap_icon_info) are appropriate additions to support screen reader and accessibility features for community icons. The strings follow the existing accessibility pattern in the file and integrate well with the PR's broader UI/accessibility improvements.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
app/src/main/res/values/strings.xml (1)
34-36: Inconsistent spacing in Korean text—consider adding space before "입니다".The three new Community string entries (lines 34–36) read "X 아이콘입니다" (no space before "입니다"), which is inconsistent with existing similar entries on lines 9–11, which follow the pattern "X 아이콘 입니다" (with space). For consistency with the existing "아이콘" entries, consider updating these to include a space.
Apply this diff to align with existing entries:
- <string name="community_user_icon_info">유저 아이콘입니다.</string> - <string name="community_like_icon_info">좋아요 아이콘입니다.</string> - <string name="community_scrap_icon_info">스크랩 아이콘입니다.</string> + <string name="community_user_icon_info">유저 아이콘 입니다.</string> + <string name="community_like_icon_info">좋아요 아이콘 입니다.</string> + <string name="community_scrap_icon_info">스크랩 아이콘 입니다.</string>
2 participants
개요
[[Chore] 16KB 페이지 지원을 위한 lib 버전 업그레이드]
PR 유형
어떤 변경 사항이 있나요?
Summary by CodeRabbit
New Features
Improvements
Chores