-
Notifications
You must be signed in to change notification settings - Fork 47
feat: Implement fullscreen WebView for vshare with theme auto-adaptation #153
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
- Add VshareWebActivity for fullscreen display of vshare page - Implement automatic light/dark theme adaptation based on app's current mode - Replace browser opening with native WebView experience - Add progress bar for loading indication - Enable JavaScript and DOM storage for proper page functionality - Handle back navigation within WebView 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
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
Implements an in‑app fullscreen WebView for the vshare feature with automatic light/dark theme adaptation instead of launching an external browser.
- Adds VshareWebActivity with WebView setup, theme parameter injection, and JS-based theme enforcement.
- Replaces external browser navigation with the new activity and updates manifest and layout.
- Provides loading feedback via a horizontal ProgressBar.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| app/src/main/res/layout/activity_vshare_web.xml | New layout containing fullscreen WebView and a horizontal progress bar. |
| app/src/main/java/me/ghui/v2er/module/vshare/VshareWebActivity.java | New activity implementing fullscreen WebView, theme parameter + JS injection, navigation handling, and cleanup. |
| app/src/main/java/me/ghui/v2er/module/home/MainActivity.java | Replaces external browser launch with opening the new VshareWebActivity. |
| app/src/main/AndroidManifest.xml | Registers VshareWebActivity with a specific theme and configChanges. |
| // Keep navigation within the WebView | ||
| view.loadUrl(request.getUrl().toString()); | ||
| return true; |
Copilot
AI
Oct 16, 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.
Returning true after calling view.loadUrl() forces manual handling of all navigations and can break POST requests or cause duplicate loads; you should return false and let the WebView handle standard navigation unless you have a specific interception case.
| // Keep navigation within the WebView | |
| view.loadUrl(request.getUrl().toString()); | |
| return true; | |
| // Let the WebView handle the navigation | |
| return false; |
|
|
||
| // Enable DOM storage | ||
| settings.setDomStorageEnabled(true); | ||
|
|
Copilot
AI
Oct 16, 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.
JavaScript + DOM storage are enabled for remote content without tightening other surfaces; consider explicitly disabling file and content access (settings.setAllowFileAccess(false); settings.setAllowContentAccess(false);) and enabling Safe Browsing (if API >= 26) to reduce attack surface.
| // Disable file and content access for security | |
| settings.setAllowFileAccess(false); | |
| settings.setAllowContentAccess(false); | |
| // Enable Safe Browsing if API >= 26 | |
| if (android.os.Build.VERSION.SDK_INT >= android.os.Build.VERSION_CODES.O) { | |
| WebView.enableSafeBrowsing(mWebView.getContext()); | |
| settings.setSafeBrowsingEnabled(true); | |
| } |
| public static void open(Context context) { | ||
| Intent intent = new Intent(context, VshareWebActivity.class); | ||
| // Append theme parameter based on current app theme | ||
| String url = VSHARE_BASE_URL; | ||
| if (DarkModelUtils.isDarkMode()) { | ||
| url += "?theme=dark"; | ||
| } else { | ||
| url += "?theme=light"; | ||
| } | ||
| intent.putExtra(KEY_URL, url); |
Copilot
AI
Oct 16, 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.
The theme-specific URL is computed only once at launch; if the system switches between light/dark while the activity is alive, the existing intent extra is reused and may produce a mismatched initial rendering; recompute the URL in onCreate (based on current mode) or handle uiMode changes by overriding onConfigurationChanged and reloading with an updated parameter.
| @Override | ||
| public void onPageFinished(WebView view, String url) { | ||
| super.onPageFinished(view, url); | ||
| mProgressBar.setVisibility(View.GONE); |
Copilot
AI
Oct 16, 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.
[nitpick] Progress bar visibility is controlled in both onPageFinished and onProgressChanged leading to duplicated logic; consolidate visibility handling (e.g., rely solely on WebChromeClient progress events) to reduce redundancy.
| mProgressBar.setVisibility(View.GONE); |
| mWebView.setWebChromeClient(new WebChromeClient() { | ||
| @Override | ||
| public void onProgressChanged(WebView view, int newProgress) { | ||
| mProgressBar.setProgress(newProgress); | ||
| if (newProgress == 100) { | ||
| mProgressBar.setVisibility(View.GONE); | ||
| } | ||
| } |
Copilot
AI
Oct 16, 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.
[nitpick] Progress bar visibility is controlled in both onPageFinished and onProgressChanged leading to duplicated logic; consolidate visibility handling (e.g., rely solely on WebChromeClient progress events) to reduce redundancy.
| mWebView.clearCache(true); | ||
| mWebView.loadUrl("about:blank"); |
Copilot
AI
Oct 16, 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.
[nitpick] Clearing cache with true on every destroy purges shared WebView cache and degrades subsequent load performance; unless there is a privacy requirement, remove clearCache(true) (or use clearCache(false)) and avoid unnecessary about:blank navigation.
| mWebView.clearCache(true); | |
| mWebView.loadUrl("about:blank"); | |
| mWebView.clearCache(false); |
| <FrameLayout xmlns:android="http://schemas.android.com/apk/res/android" | ||
| android:layout_width="match_parent" | ||
| android:layout_height="match_parent" | ||
| android:background="?android:attr/windowBackground"> |
Copilot
AI
Oct 16, 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.
[nitpick] Using ?android:attr/windowBackground can bypass app-level theme customization (e.g., dynamic day/night palette); prefer ?attr/windowBackground to respect the app's themed attribute.
| android:background="?android:attr/windowBackground"> | |
| android:background="?attr/windowBackground"> |
- Fix navigation handling: Return false in shouldOverrideUrlLoading to let WebView handle standard navigation - Add security settings: Disable file/content access and enable Safe Browsing for API >= 26 - Fix theme switching: Compute URL in onCreate to handle live theme changes - Remove redundant progress bar logic: Consolidate visibility handling in WebChromeClient - Optimize cache handling: Use clearCache(false) to avoid purging shared cache - Fix layout theming: Use app-themed attribute (page_bg_color) instead of android:attr All issues identified by GitHub Copilot have been addressed. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- Use NoneSlideBackableTheme instead of GalleryTheme for proper theme inheritance - Remove manual SystemUI configuration code (now handled by theme) - Add proper edge-to-edge display with LAYOUT_FULLSCREEN and LAYOUT_HIDE_NAVIGATION flags - Fix layout background to use transparent color compatible with NoneSlideBackableTheme - Set status bar icon color dynamically based on dark/light theme Benefits: - Reduces code duplication by leveraging existing theme infrastructure - Automatic theme switching support (follows app's day/night mode) - Consistent UI behavior with MainActivity - Simpler and more maintainable code 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Summary
Changes
VshareWebActivityfor fullscreen WebView displayTest Plan
🤖 Generated with Claude Code