-
Notifications
You must be signed in to change notification settings - Fork 245
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
fix: bound custom fontsizes between 6 and 64 #1577
Conversation
WalkthroughThe pull request introduces modifications to the The implementation updates the way font sizes are processed by applying the The modification maintains the existing component structure and rendering logic, focusing solely on improving the robustness of font size value management. The import statement is expanded to include the These changes represent a targeted enhancement to the font size handling mechanism, introducing additional validation and constraint to the Markdown element's styling properties. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🔇 Additional comments (3)frontend/app/element/markdown.tsx (3)
The boundNumber utility is appropriately imported alongside existing utilities.
There's a discrepancy between the PR objectives (which mention a range of 4-64px) and the implementation (which uses 6-64px). While the implementation is functionally correct, we should ensure this difference is intentional. - mergedStyle["--markdown-font-size"] = `${boundNumber(fontSizeOverride, 6, 64)}px`;
+ mergedStyle["--markdown-font-size"] = `${boundNumber(fontSizeOverride, 4, 64)}px`;
- mergedStyle["--markdown-fixed-font-size"] = `${boundNumber(fixedFontSizeOverride, 6, 64)}px`;
+ mergedStyle["--markdown-fixed-font-size"] = `${boundNumber(fixedFontSizeOverride, 4, 64)}px`; Could you clarify if using 6px as the minimum (instead of 4px) was intentional? If not, I've provided a diff above to align with the PR objectives.
Let's ensure these font size bounds are consistently applied wherever font sizes are manipulated. ✅ Verification successfulFont size bounds are consistently applied through a shared utility function The codebase shows consistent usage of font size bounds through the
🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Search for other instances of font size manipulation in CSS/SCSS files
rg -t css -t scss "font-size|--markdown-font-size|--markdown-fixed-font-size"
# Search for other potential uses of boundNumber with font sizes
rg "boundNumber.*font.*size"
Length of output: 139 Script: #!/bin/bash
# Search for font size related patterns in all files
rg "font-size|--markdown-font-size|--markdown-fixed-font-size"
# Search for boundNumber function usage and definition
rg "boundNumber" -A 2
# Search for font size related CSS variables
ast-grep --pattern 'var(--markdown-font-size)'
ast-grep --pattern 'var(--markdown-fixed-font-size)'
Length of output: 117019 Script: #!/bin/bash
# Search for font size related CSS variables and their usage
rg "font-size|--markdown-font-size|--markdown-fixed-font-size" frontend/
# Search for boundNumber function definition and usage
rg "boundNumber" frontend/ -A 3
Length of output: 12180 🪧 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
CodeRabbit Configuration File (
|
Adds bounding for markdown (and AI) fontsize and fixedfontsize.