Feature/colorshift upstream pr84 #12
Conversation
'ColorShift' is a neew option which just changes the overall text color (cherry picked from commit d4f2cad)
Updated dev dependency to satisfy Rollup plugin version constraints required by @rollup/plugin-commonjs@^28
|
Caution Review failedThe pull request is closed. WalkthroughAdds a new "colorshift" highlighter style across plugin, settings, and styles. Updates mark element generation to set both color and background when not using CSS classes. Toggles a body class for colorshift during onload and in updateStyle. Extends settings options and demos. Adjusts CSS for color inheritance and gradients. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant P as Plugin
participant S as Settings
participant D as DOM/CSS
U->>P: Load plugin
P->>S: Read highlighterStyle
alt highlighterStyle == "colorshift"
P->>D: Add body class .highlightr-colorshift
else
P->>D: Ensure .highlightr-colorshift not present
end
U->>P: Change highlighter style (settings)
P->>S: Persist new style
P->>D: updateStyle()
alt style == "colorshift"
P->>D: Add .highlightr-colorshift
else
P->>D: Remove .highlightr-colorshift
end
note over P,D: Rendering highlights
P->>D: Insert <mark>...</mark>\n- If not css-classes: set style color + background\n- Else rely on CSS rules
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
✨ Finishing touches🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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: 3
🧹 Nitpick comments (5)
styles.css (1)
46-50: Sync demo with CSS: Floating gradient now stops at 14%.
Update the inline demo in src/settings/settingsTab.ts to 14% to match this change.src/settings/settingsTab.ts (4)
11-15: Follow base-url import guideline.
Use base-url import instead of relative path.-import { - HIGHLIGHTER_METHODS, - HIGHLIGHTER_STYLES, - CONTEXT_MENU_PALETTE, -} from "./settingsData"; +import { + HIGHLIGHTER_METHODS, + HIGHLIGHTER_STYLES, + CONTEXT_MENU_PALETTE, +} from "src/settings/settingsData";
70-75: Fix user-facing typo (“overriden” → “overridden”).-`Depending on your design aesthetic, you may want to customize the style of your highlights. Choose from an assortment of different highlighter styles by using the dropdown. Depending on your theme, this plugin's CSS may be overriden.` +`Depending on your design aesthetic, you may want to customize the style of your highlights. Choose from an assortment of different highlighter styles by using the dropdown. Depending on your theme, this plugin's CSS may be overridden.`
93-98: Sync Floating demo with CSS (28% → 14%).- <span style="background:#93C0FFCC;--floating-background: var(--background-primary);border-radius: 0;padding-bottom: 5px;background-image: linear-gradient(360deg,rgba(255, 255, 255, 0) 28%,var(--floating-background) 28%) !important;">Floating</span> + <span style="background:#93C0FFCC;--floating-background: var(--background-primary);border-radius: 0;padding-bottom: 5px;background-image: linear-gradient(360deg,rgba(255, 255, 255, 0) 14%,var(--floating-background) 14%) !important;">Floating</span>
229-252: Avoid ‘any’: type the click event.
Use MouseEvent to satisfy “prefer explicit types”.- .onClick(async (buttonEl: any) => { + .onClick(async (evt: MouseEvent) => { let color = colorInput.inputEl.value.replace(" ", "-"); let value = valueInput.inputEl.value; if (color && value) { if (!this.plugin.settings.highlighterOrder.includes(color)) { this.plugin.settings.highlighterOrder.push(color); this.plugin.settings.highlighters[color] = value; setTimeout(() => { dispatchEvent(new Event("Highlightr-NewCommand")); }, 100); await this.plugin.saveSettings(); this.display(); } else { - buttonEl.stopImmediatePropagation(); + evt.stopImmediatePropagation(); new Notice("This color already exists"); } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (2)
assets/colorshift-update.pngis excluded by!**/*.pngpackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
package.json(1 hunks)src/plugin/main.ts(2 hunks)src/settings/settingsData.ts(1 hunks)src/settings/settingsTab.ts(1 hunks)styles.css(5 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
{manifest.json,package.json}
📄 CodeRabbit inference engine (AGENTS.md)
When bumping versions, keep manifest.json and package.json versions in sync
Files:
package.json
src/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.ts: Prefer explicit types over usingany
Code formatting: 2-space indentation, use semicolons, use double quotes, avoid trailing commas
Use base-url imports fromsrc/...instead of relative paths
Files:
src/settings/settingsTab.tssrc/settings/settingsData.tssrc/plugin/main.ts
src/{ui,settings,utils,icons}/**
📄 CodeRabbit inference engine (AGENTS.md)
Place UI, settings, helper utilities, and icons in their respective directories:
src/ui/,src/settings/,src/utils/,src/icons/
Files:
src/settings/settingsTab.tssrc/settings/settingsData.ts
{main.js,styles.css}
📄 CodeRabbit inference engine (AGENTS.md)
Build outputs must be
main.jsandstyles.cssat the repository root
Files:
styles.css
src/plugin/main.ts
📄 CodeRabbit inference engine (AGENTS.md)
Entry point is
src/plugin/main.tsand should contain plugin startup/wiring
Files:
src/plugin/main.ts
🔇 Additional comments (5)
styles.css (3)
13-13: LGTM: prevent unintended text color overrides.
Using color: unset !important on lowlight marks restores theme/inherited color.
81-81: LGTM: rounded text color inheritance.
Matches intent to avoid forcing foreground color.
110-111: LGTM: realistic text color inheritance.
Consistent with other styles.src/settings/settingsData.ts (1)
7-8: LGTM: added “colorshift” style key.
Data-only change; no API impact.src/plugin/main.ts (1)
200-203: LGTM: colorshift body class toggle.
Ensures runtime switch via updateStyle; pair with per-color text rules for “css-classes” method (see styles/createStyles note).Please verify colorshift behavior in both methods:
- inline-styles + colorshift: text colored, background removed.
- css-classes + colorshift: text colored via class; background removed.
Before merging
Wait two weeks for comments from the original PR creator, just in case
Summary
This PR adopts the upstream "ColorShift" feature from the original Highlightr-Plugin repository.
What's Changed
✨ New Feature: ColorShift Highlighter Style
Adds a new "ColorShift" highlighting option that changes the text color instead of the background. This provides an alternative highlighting style that some users may find more subtle or readable.
📝 Changes Included
colorproperty and togglehighlightr-colorshiftclass on document body🔧 Additional Updates
@rollup/plugin-node-resolveto^15to satisfy Rollup plugin version constraintsTesting
npm run buildCredits
Full credit goes to @Jayydoodle for the original implementation. This fork preserves the original authorship and includes proper attribution.
Summary by CodeRabbit
New Features
Style