Feature/if no cjk skip #239
Merged
Conversation
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
- Implement pageHasCJK() method to detect CJK characters in page content - Add watchForCJKContent() for dynamic monitoring of CJK content - Skip spacing operations when no CJK content is present - Add comprehensive Playwright tests for CJK detection - Update TODO.md to mark CJK detection feature as complete This optimization improves performance by avoiding unnecessary processing on pages that don't contain Chinese, Japanese, or Korean characters. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Add smartAutoSpacingPage() that skips processing if no CJK content detected - Refactor autoSpacingPage() to use config objects instead of positional parameters - Add Ms suffix to time-related properties for clarity (pageDelayMs, nodeDelayMs, nodeMaxWaitMs) - Implement watchForCJKContent() to monitor for dynamically added CJK content - Breaking change: autoSpacingPage() now requires config object 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Move public methods before protected methods following TypeScript conventions - Reorganize autoSpacingPage and smartAutoSpacingPage for better logical flow - Add hasCJK() and watchForCJKContent() methods for efficient CJK detection - Skip spacing on pages without CJK content to improve performance - Set up observer to detect dynamically loaded CJK content - Add SmartAutoSpacingPageConfig interface with sampleSize parameter - Rename test file to better reflect smartAutoSpacingPage functionality 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Changed watchForCJKContent to accept AutoSpacingPageConfig instead of individual parameters - Preserved all config settings when restarting autoSpacingPage after CJK detection - Adjusted test timing to ensure CJK detection and spacing completion 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Store observers as class properties for better lifecycle management - Rename methods for camelCase consistency: hasCJK → hasCjk, watchForCJKContent → setupCjkObserver - Add stopAutoSpacingPage() method to cleanly disconnect all observers - Reorganize property declarations for better readability - Ensure only one observer of each type runs at a time 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Made all AutoSpacingPageConfig and SmartAutoSpacingPageConfig properties optional - Refactored autoSpacingPage and smartAutoSpacingPage to use destructuring with default values - Updated setupCjkObserver to only destructure used parameters - Removed need for Partial<> type wrapper and object spread for configs 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Created new setupAutoSpacingPageObserver method to encapsulate MutationObserver setup logic - Improved code organization by separating observer setup from autoSpacingPage logic - Enhanced modularity and maintainability of the codebase - Moved methods to better logical order in the class 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Add tests/ directory to ESLint configuration - Create tsconfig.eslint.json to include test files for TypeScript parsing - Fix all test imports to use node: protocol prefix - Merge CJK Detection tests into main pangu.playwright.ts file - Remove duplicate pangu.smartAutoSpacingPage.playwright.ts file 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Move all hardcoded HTML fragments to fixture files
- Remove unnecessary comments throughout test file
- Simplify beforeEach by using path option directly instead of reading file content
- Remove redundant page.goto('about:blank') call
- Add clarifying comment for CJK Detection tests that need to reload pangu.js
🤖 Generated with [Claude Code](https://claude.ai/code)
Co-Authored-By: Claude <[email protected]>
- Rename all test titles to follow "should xxx" convention in English - Fix TypeScript errors in UMD import tests with @ts-expect-error - Import UMD types directly and use named imports for path utilities - Rename imports.playwright.ts to import.umd.playwright.ts for clarity 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Rename import.umd.playwright.ts back to imports.playwright.ts - Rename urlpattern.chrome.playwright.ts to urlpattern.playwright.ts - Reorder imports in tests/node/imports.test.ts for consistency - Simplify browser UMD import tests with cleaner structure 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
vinta
force-pushed
the
feature/if-no-cjk-skip
branch
from
fbad80b to
042173e
Compare
The CJK detection feature has been fully implemented and integrated into the codebase. The research document is no longer needed. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Add is_enable_detect_cjk setting to control smart spacing behavior - Update content script to use smartAutoSpacingPage() when enabled - Add toggle in options page for "只在有中文的頁面才招喚空格之神" - Add i18n messages for new smart mode labels - Default to false (always process pages) for backward compatibility When enabled, the extension only processes pages with CJK content, improving performance on non-CJK pages. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Wrap toggles in .options-toggle-section divs for consistent spacing - Add blue color scheme for active toggle states - Match toggle label styling with popup page 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Replace inline message display with floating notification overlay - Add slide-down animation and shadow effect for hovering appearance - Include close button for manual dismissal - Support info, error, and success message types with distinct colors 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Remove close button, make entire notification clickable - Remove hover effect for cleaner appearance - Fix button re-enable issue when manually dismissing notification - Store callback to ensure button is properly re-enabled on any dismiss method 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Track currently playing audio in sound utility - Add stopSound() function to pause and reset audio - Call stopSound() when hiding notification (manual or auto dismiss) - Prevents sounds from continuing after error/success messages are closed 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Add mute sound effects toggle between spacing mode and status indicator - Sync mute state with options page settings - Play confirmation sound when turning off mute - Allow users to control sounds without opening full settings 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
…e import issue - Update build:extension script to copy pangu.umd.js from dist to extension vendors - Fix "Cannot use import statement outside a module" error by inlining DEFAULT_SETTINGS in content script - Update watch script to run both lib and extension watch commands in parallel - Update watch:extension to properly watch for pangu.umd.js changes 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Create default-settings.json as single source of truth for settings - Import JSON in settings.ts and content-script.ts with type assertions - Use multiBuildPlugin pattern to build content script as IIFE in extension config - JSON is automatically inlined during content script IIFE build - Maintain consistent naming with main vite.config.ts This approach avoids ES module issues in content scripts while keeping a single source of configuration. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Remove default-settings.json file - Define DEFAULT_SETTINGS directly in settings.ts - Remove unused detect_cjk_help locale message - Update dist files to reflect the changes 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Remove unnecessary comments from settings.ts - Consolidate blacklist array formatting - Update build:extension script order in package.json - Clean up sound.ts formatting 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
…onfig - Remove tests/ from exclude list in main tsconfig.json - Update ESLint to use main tsconfig.json directly - Delete redundant tsconfig.eslint.json file - Reduces configuration duplication since main tsconfig already has noEmit: true 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Replace wildcard imports with specific named imports in test files - Import only used functions from node:fs and node:path modules - Improves code clarity and tree-shaking potential 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Update all test descriptions in tests/shared/index.test.ts - Change from Chinese descriptions to English with 'should' prefix - Improves consistency and readability of test output 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Remove explicit folder paths from lint commands in package.json - Add vendor and build directories to ESLint ignores - Rely on centralized ESLint config for file patterns 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
There was a problem hiding this comment.
Pull Request Overview
This PR implements a feature to skip spacing on pages that lack CJK content. Key changes include adding CJK detection via the new hasCjk() and smartAutoSpacingPage() methods in the BrowserPangu class, updating tests and UI elements to support this behavior, and incorporating related configuration and build adjustments.
Reviewed Changes
Copilot reviewed 32 out of 50 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/browser/pangu.ts | Introduces new CJK detection and auto-spacing logic (hasCjk, smartAutoSpacingPage, and observer setup) |
| tests/browser/pangu.playwright.ts | Adds tests for CJK detection, ensuring spacing is skipped when no CJK content is present |
| browser_extensions/chrome/src/utils/settings.ts | Adds a new setting property (is_enable_detect_cjk) to control CJK detection |
| browser_extensions/chrome/src/options.ts & popup.ts | Update UI and event listeners to support the new CJK detection and configuration options |
| Other test and build configuration files | Update import paths and minor refactoring to support new feature |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
No description provided.