-
-
Notifications
You must be signed in to change notification settings - Fork 129
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
新增功能:为 Markdown 内容中的 <del> 标签添加遮罩效果 #464
base: dev
Are you sure you want to change the base?
Conversation
fix: link icon
chore: bump version to 2.8.1
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 Walkthrough📝 Walkthrough📝 Walkthrough📝 WalkthroughWalkthroughThe changes introduce a new configuration option Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Hexo
participant Filter
participant CSS
User->>Hexo: Submit post with <del> tags
Hexo->>Filter: Trigger after_post_render
Filter->>Hexo: Check delete_mask configuration
alt delete_mask is true
Filter->>Hexo: Wrap <del> content with .mask class
else delete_mask is false
Filter->>Hexo: Leave <del> content unchanged
end
Hexo->>User: Render post with modified content
User->>CSS: Hover over <del> content
CSS->>User: Apply hover effects and transitions
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
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.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (3)
scripts/filters/delete-mask-handle.js (2)
5-27
: Consider performance optimization for large content.The current implementation processes all content with regex replacement, which might impact performance for large posts.
Consider these optimizations:
- Early return if
delete_mask
is disabled- Quick check for
<del>
presence before regex processing- Cache compiled regex
Example implementation:
hexo.extend.filter.register('after_post_render', function (data) { const deleteMaskEnabled = this.theme.config?.articles?.style?.delete_mask === true; // Early return if feature is disabled if (!deleteMaskEnabled) return data; // Quick check before expensive regex if (!data.content.includes('<del')) return data; try { // Cache compiled regex const regPureDelTag = /<del(?:\s+[^>]*)?>((?:(?!<\/?del[\s>])[^])*)<\/del>/g; data.content = data.content.replace(regPureDelTag, (_, html) => `<del class="mask">${html}</del>` ); } catch (error) { hexo.log.error('Error processing delete masks:', error); } return data; }, 0);
26-26
: Document the priority parameter.The
0
priority parameter determines when this filter runs relative to other filters. Consider adding a comment explaining this choice.Add documentation:
}, + // Priority 0 ensures this runs at a neutral priority relative to other post-render filters 0
source/css/common/markdown.styl (1)
213-229
: Consider accessibility and RTL support for the mask effect.The masking implementation looks solid and matches the PR objectives well. However, consider these enhancements:
- Add screen reader support:
del.mask text-decoration none !important + aria-label "Masked content. Hover to reveal." transition .2s cubic-bezier(.25, .46, .45, .94) color, .2s cubic-bezier(.25, .46, .45, .94) background
- Add RTL support:
del.mask text-decoration none !important transition .2s cubic-bezier(.25, .46, .45, .94) color, .2s cubic-bezier(.25, .46, .45, .94) background + [dir="rtl"] & + /* Add any RTL-specific adjustments if needed */
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (3)
_config.yml
(1 hunks)scripts/filters/delete-mask-handle.js
(1 hunks)source/css/common/markdown.styl
(2 hunks)
🧰 Additional context used
🪛 Biome
scripts/filters/delete-mask-handle.js
[error] 2-3: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.
(lint/suspicious/noRedundantUseStrict)
🔇 Additional comments (1)
source/css/common/markdown.styl (1)
336-336
: Verify the impact on math formula rendering.
This change to display: inline-block
for mjx-container svg
appears unrelated to the masking feature. Please verify that this doesn't affect the rendering of mathematical formulas.
✅ Verification successful
Let me check the content of these files to understand the context of MathJax usage and any related styling requirements.
The display: inline-block
change for mjx-container svg
is safe and actually improves math formula rendering
The change is actually beneficial for math formula rendering. From the context found:
- The parent
mjx-container
already has proper display settings:display: inline
for inline formulasdisplay: block
withtext-align: center
for block-level formulas
- The
inline-block
on SVG elements allows them to be centered properly within their containers, as indicated by the code comment.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any math-related test files or documentation
rg -g '!node_modules' -l 'mjx-container|mathjax' .
# Search for any related issues or PRs
gh issue list --search "mjx-container mathjax"
gh pr list --search "mjx-container mathjax"
Length of output: 229
Script:
#!/bin/bash
# Check README and CSS content for MathJax context
echo "=== README.md MathJax section ==="
rg -A 5 -B 5 'mathjax|mjx-container' README.md
echo -e "\n=== markdown.styl MathJax styles ==="
rg -A 5 -B 5 'mjx-container' source/css/common/markdown.styl
# Check if there are any test files or examples with math formulas
echo -e "\n=== Files with math examples ==="
fd -e md -e html . -x rg -l '\$\$|\\\[|\\\(' {} \;
Length of output: 1465
/* delete mask */ | ||
|
||
"use strict"; | ||
|
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.
🛠️ Refactor suggestion
Remove redundant 'use strict' directive and consider comment localization.
- The 'use strict' directive is redundant in JavaScript modules as they are automatically in strict mode.
- Consider using Chinese comments to maintain consistency with the PR title and intended audience.
Apply this diff:
-/* delete mask */
+/* 删除标签遮罩效果 */
-
-"use strict";
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
/* delete mask */ | |
"use strict"; | |
/* 删除标签遮罩效果 */ |
🧰 Tools
🪛 Biome
[error] 2-3: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.
(lint/suspicious/noRedundantUseStrict)
const theme = this.theme; | ||
|
||
// 处理del标签的代码 | ||
const regPureDelTag = /<del>([^<]*)<\/del>/gim; |
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.
🛠️ Refactor suggestion
Enhance regex pattern for better HTML compatibility.
The current regex pattern /<del>([^<]*)<\/del>/gim
has limitations:
- Doesn't handle HTML attributes in
<del>
tags - Doesn't support nested HTML elements
- Case-insensitive flag (
i
) is unnecessary for HTML tags
Consider using this improved pattern:
-const regPureDelTag = /<del>([^<]*)<\/del>/gim;
+const regPureDelTag = /<del(?:\s+[^>]*)?>((?:(?!<\/?del[\s>])[^])*)<\/del>/g;
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
const regPureDelTag = /<del>([^<]*)<\/del>/gim; | |
const regPureDelTag = /<del(?:\s+[^>]*)?>((?:(?!<\/?del[\s>])[^])*)<\/del>/g; |
data.content = data.content.replace( | ||
regPureDelTag, | ||
function (match, html) { | ||
// 只有在配置为true时才添加mask类 | ||
if (theme.config.articles.style.delete_mask === true) { | ||
return `<del class="mask">${html}</del>`; | ||
} | ||
return match; | ||
} | ||
); |
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.
🛠️ Refactor suggestion
Add error handling and theme config validation.
The current implementation lacks error handling and config validation:
- No validation of
theme.config.articles.style
- No error handling for malformed HTML
Consider this safer implementation:
- data.content = data.content.replace(
- regPureDelTag,
- function (match, html) {
- // 只有在配置为true时才添加mask类
- if (theme.config.articles.style.delete_mask === true) {
- return `<del class="mask">${html}</del>`;
- }
- return match;
- }
- );
+ try {
+ const deleteMaskEnabled = theme.config?.articles?.style?.delete_mask === true;
+ data.content = data.content.replace(
+ regPureDelTag,
+ function (match, html) {
+ // 只有在配置为true时才添加mask类
+ return deleteMaskEnabled ? `<del class="mask">${html}</del>` : match;
+ }
+ );
+ } catch (error) {
+ hexo.log.error('Error processing delete masks:', error);
+ return data;
+ }
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
data.content = data.content.replace( | |
regPureDelTag, | |
function (match, html) { | |
// 只有在配置为true时才添加mask类 | |
if (theme.config.articles.style.delete_mask === true) { | |
return `<del class="mask">${html}</del>`; | |
} | |
return match; | |
} | |
); | |
try { | |
const deleteMaskEnabled = theme.config?.articles?.style?.delete_mask === true; | |
data.content = data.content.replace( | |
regPureDelTag, | |
function (match, html) { | |
// 只有在配置为true时才添加mask类 | |
return deleteMaskEnabled ? `<del class="mask">${html}</del>` : match; | |
} | |
); | |
} catch (error) { | |
hexo.log.error('Error processing delete masks:', error); | |
return data; | |
} |
@@ -275,6 +275,7 @@ articles: | |||
image_alignment: center # image alignment. left, center | |||
image_caption: false # Whether to display image caption | |||
link_icon: true # Whether to display link icon | |||
delete_mask: false # Add mask effect to <del> tags, hiding content by default and revealing on hover |
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.
💡 Codebase verification
Documentation update needed for the new delete_mask feature
The new delete_mask
configuration option needs documentation as it's not currently documented in any of the existing files. Please add documentation in the following locations:
- Add the configuration option to the theme configuration section in
README.md
- Include it in the translated README files (
README_zh-CN.md
andREADME_zh-TW.md
) - Document the feature's behavior and usage in the configuration guide
🔗 Analysis chain
Configuration looks good, but needs documentation update
The new delete_mask
configuration option is well-placed under the articles.style
section and properly defaults to false
for backward compatibility. However, since this introduces a new user-facing feature, it should be documented in the theme's documentation.
Let's verify if the documentation needs to be updated:
Would you like me to help draft the documentation for this new feature? I can prepare:
- Configuration guide explaining the
delete_mask
option - Usage examples with and without the mask effect
- Screenshots demonstrating the hover behavior
Let me search for any existing documentation in other potential locations and check for related configuration files to ensure we have a complete picture.
Let me check if there's a dedicated documentation directory or theme-specific documentation files.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if documentation exists for the new feature
# Look for any documentation files that might need updating
# Search for potential documentation files
fd -e md -e mdx . | while read -r file; do
if [[ -f "$file" ]]; then
echo "Checking $file for existing documentation..."
rg -i "delete_?mask|del tag|strikethrough" "$file"
fi
done
# Check if there's a dedicated documentation site
if [[ -f "docs/README.md" ]]; then
echo "Checking documentation site..."
rg -i "delete_?mask|del tag|strikethrough" "docs/README.md"
fi
Length of output: 1364
Script:
#!/bin/bash
# Check for documentation in docs directory and theme configuration files
fd -t f -e yml -e yaml . | while read -r file; do
echo "Checking $file for related configuration..."
rg -i "delete_?mask|del tag|strikethrough" "$file"
done
# Check for any documentation in Jekyll includes/layouts
fd -t f -e html . "_includes" "_layouts" | while read -r file; do
echo "Checking template file $file..."
rg -i "delete_?mask|del tag|strikethrough" "$file"
done
Length of output: 1531
Script:
#!/bin/bash
# Look for theme documentation and configuration files
fd -t d "docs|wiki|documentation" .
# Check for theme-specific documentation files
fd -t f -e md "theme|config|setting|documentation" .
# Check the directory structure to understand the theme organization
ls -la
Length of output: 1728
lgtm,就缺个文档补充了 |
This pull request has been deployed to Vercel.
|
|
清除一些冗余代码
为 Markdown 内容中的 del 标签添加遮罩效果,可通过配置选项控制删除线内容默认隐藏,悬停时显示。
配置项启用时:
鼠标未悬停:
鼠标悬停:
Markdown内容:
配置项未启用时:
为原删除线效果。
Summary by CodeRabbit
New Features
delete_mask
configuration option to enhance the display of deleted content in articles, hiding it by default and revealing it on hover.<del>
tags in posts based on thedelete_mask
setting.Style
.mask
class with hover effects and transitions for improved visual presentation of deleted content.