SCRUM-270 SCRUM-271 feat: add terms webview#43
Merged
gdaegeun539 merged 14 commits intoMay 5, 2026
Merged
Conversation
Patch Notion's collapsed scroll container so legal pages remain visible inside the Android internal WebView. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Use public Notion URLs for WebView display while preserving the original agreement URLs sent in signup payloads. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Add AndroidX WebKit and apply runtime darkening settings so interoperated WebView content can react to dark mode changes.
📝 WalkthroughWalkthroughAdds an internal WebView flow: new composable screen hosting an Android WebView with dark-mode support and refresh/close actions, navigation route and URL encoding, settings-driven link definitions that choose internal vs external opening, a refresh drawable/icon, an external browser launcher, and the androidx-webkit dependency; SignUpTerm gains a separate display URL field. ChangesInternal WebView Feature
Sequence DiagramssequenceDiagram
actor User
participant Settings as SettingScreen
participant Nav as Navigation
participant Web as InternalWebView
participant Sys as System Browser
User->>Settings: Tap link
Settings->>Nav: request open(link.url)
alt opensInternally
Nav->>Nav: encode url
Nav->>Web: navigate(encoded url)
Web->>Web: decode, apply dark mode, load content
Web->>User: render page
else opens externally
Nav->>Sys: launch ACTION_VIEW(url)
Sys->>User: open in external browser
end
sequenceDiagram
actor User
participant Onboarding as OnboardingTermsScreen
participant Nav as Navigation
participant Web as InternalWebView
User->>Onboarding: Tap term detail
Onboarding->>Nav: createRoute(term.webViewUrl)
Nav->>Nav: encode url
Nav->>Web: navigate(encoded url)
Web->>Web: apply dark mode, load
User->>Web: tap refresh or close
Web->>Web: reload
Web->>Nav: popBackStack on close
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
gdaegeun539
commented
May 5, 2026
gdaegeun539
commented
May 5, 2026
| WebView(context).apply { | ||
| webView = this | ||
| // Notion의 full-height 레이아웃이 Android WebView에서 0px로 접히지 않도록 명시한다. | ||
| layoutParams = ViewGroup.LayoutParams( |
Member
Author
There was a problem hiding this comment.
웹뷰가 표시되는 뷰시스템의 크기를 잡지 않고 링크를 표시하면 노션 페이지의 레이아웃이 망가져서 표시되더라고요.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 04f4d64dfc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
gdaegeun539
commented
May 5, 2026
| @@ -0,0 +1,28 @@ | |||
| package com.lyrics.feelin.presentation.view.mypage.setting | |||
|
|
|||
| enum class SettingInfoLink( | |||
Member
Author
There was a problem hiding this 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: 3
🧹 Nitpick comments (1)
app/src/main/java/com/lyrics/feelin/presentation/view/mypage/setting/SettingInfoLink.kt (1)
8-27: ⚡ Quick winConsolidate legal-link URLs into one source of truth.
SERVICE_USAGE/PERSONAL_INFOURLs are duplicated here and inSignUpTerm, which can drift and show different legal documents across onboarding vs settings. Consider centralizing shared legal links in one model/constants source.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/main/java/com/lyrics/feelin/presentation/view/mypage/setting/SettingInfoLink.kt` around lines 8 - 27, SERVICE_USAGE and PERSONAL_INFO duplicate legal URLs here and in SignUpTerm causing drift; create a single source of truth (e.g., a LegalLinks object or enum) that exposes constants/properties for SERVICE_USAGE_URL and PERSONAL_INFO_URL, move the URLs into that new LegalLinks, and update SettingInfoLink (entries SERVICE_USAGE, PERSONAL_INFO) and SignUpTerm to reference LegalLinks.SERVICE_USAGE_URL and LegalLinks.PERSONAL_INFO_URL respectively so both consumers use the same centralized constants.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/main/java/com/lyrics/feelin/navigation/FeelinNavHost.kt`:
- Around line 275-284: In the onExternalBrowserClick lambda in FeelinNavHost
(the block that calls context.startActivity(Intent(Intent.ACTION_VIEW,
url.toUri()))), keep the existing Log.w for ActivityNotFoundException but also
show a user-facing Toast (e.g., Toast.makeText(context, "No browser found to
open link", Toast.LENGTH_SHORT).show()) inside the onFailure branch when
throwable is ActivityNotFoundException; do not rethrow in that branch so the
Toast is the visible feedback and preserve current behavior for other
exceptions.
In
`@app/src/main/java/com/lyrics/feelin/presentation/view/webview/InternalWebViewScreen.kt`:
- Around line 68-69: The contentDescription for the refresh control in
InternalWebViewScreen is hardcoded; replace the literal "Refresh" used in the
contentDescription property with a localized string resource (use
stringResource(R.string.refresh) inside InternalWebViewScreen) and add a
corresponding entry named "refresh" to your strings.xml (and translated Korean
strings.xml) so the accessibility text is localized across locales.
- Around line 83-97: The WebView is created inside the WebView(context).apply
block in InternalWebViewScreen but no WebViewClient is set, so navigations open
the system browser; set webView.webViewClient (inside the same
WebView(context).apply block) to a WebViewClient implementation that keeps
navigation in-app — e.g., override shouldOverrideUrlLoading to load the request
in the current WebView (or return false to let the WebView handle it) and handle
redirects/errors as needed; ensure this assignment occurs before calling
loadUrl(url) so applyDarkTheme, webView variable, and loadUrl(url) continue to
work correctly.
---
Nitpick comments:
In
`@app/src/main/java/com/lyrics/feelin/presentation/view/mypage/setting/SettingInfoLink.kt`:
- Around line 8-27: SERVICE_USAGE and PERSONAL_INFO duplicate legal URLs here
and in SignUpTerm causing drift; create a single source of truth (e.g., a
LegalLinks object or enum) that exposes constants/properties for
SERVICE_USAGE_URL and PERSONAL_INFO_URL, move the URLs into that new LegalLinks,
and update SettingInfoLink (entries SERVICE_USAGE, PERSONAL_INFO) and SignUpTerm
to reference LegalLinks.SERVICE_USAGE_URL and LegalLinks.PERSONAL_INFO_URL
respectively so both consumers use the same centralized constants.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: fdc8b2b7-9a93-4e80-9af9-50d489441ea9
📒 Files selected for processing (10)
app/build.gradle.ktsapp/src/main/java/com/lyrics/feelin/core/designsystem/icon/Icons.ktapp/src/main/java/com/lyrics/feelin/core/domain/model/SignUpTerm.ktapp/src/main/java/com/lyrics/feelin/navigation/FeelinDestination.ktapp/src/main/java/com/lyrics/feelin/navigation/FeelinNavHost.ktapp/src/main/java/com/lyrics/feelin/presentation/view/mypage/setting/SettingInfoLink.ktapp/src/main/java/com/lyrics/feelin/presentation/view/mypage/setting/SettingScreen.ktapp/src/main/java/com/lyrics/feelin/presentation/view/webview/InternalWebViewScreen.ktapp/src/main/res/drawable/refresh.xmlgradle/libs.versions.toml
Assign a WebViewClient so policy page redirects and link navigation stay inside the internal WebView instead of falling back to the system browser.
Move external browser launch handling out of FeelinNavHost so the navigation graph only wires link actions to presentation utilities. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Member
Author
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e9db31a1ae
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/main/java/com/lyrics/feelin/navigation/FeelinNavHost.kt`:
- Around line 291-299: The navArgument currently sets defaultValue = "" which
masks missing URLs; change the argument to be nullable (remove the empty default
and set nullable = true on the navArgument for
FeelinDestination.InternalWebView.UrlArgument) so
backStackEntry.arguments?.getString(FeelinDestination.InternalWebView.UrlArgument)
can return null, then add an explicit guard at the call site in FeelinNavHost
(where you read backStackEntry.arguments?.getString(...).orEmpty()) to check for
null/blank (e.g., if (url.isNullOrBlank()) { show error UI / navigateUp / log
and return }) instead of silently loading an empty WebView.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 776bd6c8-860a-4daf-a3e6-d906c9ce5f72
📒 Files selected for processing (3)
app/src/main/java/com/lyrics/feelin/navigation/FeelinNavHost.ktapp/src/main/java/com/lyrics/feelin/presentation/util/ExternalBrowserLauncher.ktapp/src/main/java/com/lyrics/feelin/presentation/view/webview/InternalWebViewScreen.kt
✅ Files skipped from review due to trivial changes (2)
- app/src/main/java/com/lyrics/feelin/presentation/util/ExternalBrowserLauncher.kt
- app/src/main/java/com/lyrics/feelin/presentation/view/webview/InternalWebViewScreen.kt
Comment on lines
+291
to
+299
| navArgument(FeelinDestination.InternalWebView.UrlArgument) { | ||
| type = NavType.StringType | ||
| defaultValue = "" | ||
| } | ||
| ), | ||
| ) { backStackEntry -> | ||
| val url = backStackEntry.arguments | ||
| ?.getString(FeelinDestination.InternalWebView.UrlArgument) | ||
| .orEmpty() |
There was a problem hiding this comment.
Empty-URL fallback will silently load a blank WebView.
defaultValue = "" means any navigation to this destination without the URL argument (e.g., malformed deep link) gives InternalWebViewScreen an empty string, resulting in a blank, error-free WebView. Prefer explicit null handling so the screen can bail out early or show an error.
🛡️ Proposed fix
navArgument(FeelinDestination.InternalWebView.UrlArgument) {
type = NavType.StringType
- defaultValue = ""
+ nullable = true
+ defaultValue = null
}Then guard at the call site:
-val url = backStackEntry.arguments
- ?.getString(FeelinDestination.InternalWebView.UrlArgument)
- .orEmpty()
-
-InternalWebViewScreen(
- url = url,
- onCloseClick = { navController.popBackStack() },
-)
+val url = backStackEntry.arguments
+ ?.getString(FeelinDestination.InternalWebView.UrlArgument)
+
+if (url.isNullOrBlank()) {
+ navController.popBackStack()
+} else {
+ InternalWebViewScreen(
+ url = url,
+ onCloseClick = { navController.popBackStack() },
+ )
+}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/src/main/java/com/lyrics/feelin/navigation/FeelinNavHost.kt` around lines
291 - 299, The navArgument currently sets defaultValue = "" which masks missing
URLs; change the argument to be nullable (remove the empty default and set
nullable = true on the navArgument for
FeelinDestination.InternalWebView.UrlArgument) so
backStackEntry.arguments?.getString(FeelinDestination.InternalWebView.UrlArgument)
can return null, then add an explicit guard at the call site in FeelinNavHost
(where you read backStackEntry.arguments?.getString(...).orEmpty()) to check for
null/blank (e.g., if (url.isNullOrBlank()) { show error UI / navigateUp / log
and return }) instead of silently loading an empty WebView.
Member
Author
|
빈 웹뷰 minor issue는 실제 문제 발생시 수정하고 우선 병합하겠습니다. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Please check if the PR fulfills these requirements
What kind of change does this PR introduce?
What is the current behavior?
약관, 개인정보처리방침 등의 외부 링크가 연결되지 않은 상태입니다.
What is the new behavior (if this is a feature change)?
Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)
No breaking changes.
ScreenShots
회원가입 온보딩
Screen_Recording_20260505_112940_Feelin.mp4
설정화면
Screen_Recording_20260505_113005_Feelin.mp4
외부 브라우저를 통해 링크를 표시하는데, 브라우저가 없는 경우
Other information:
노션 웹뷰를 다크모드에 대응하기 위해서는 View 시스템 테마를 다크모드에 대응하도록 수정해야 하더라고요. 해당 부분은 이 브랜치의 구현을 넘는 것 같아 이슈로 등록하고 수정하겠습니다.
Summary by CodeRabbit