refactor: improve blockquoteCompiler logic #2624
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the blockquote compiler by removing unnecessary code and simplifying the return statement. The changes improve code cleanliness without altering functionality.
- Removed logic for filtering empty paragraphs from blockquote tokens
- Simplified the return statement by eliminating an intermediate variable
Comments suppressed due to low confidence (1)
src/core/render/compiler/blockquote.js:1
- The removal of the empty paragraph filtering logic should be verified with test coverage to ensure this doesn't introduce regressions in blockquote rendering, particularly for callout blocks with empty leading paragraphs.
export const blockquoteCompiler = ({ renderer }) =>
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Thanks very much for taking a look at this @sy-records ! Unfortunately another minor issue has appeared with this fix, here is the updated Codebox: While the first line is now displayed, there seems to be an extra line now displayed before it. For example, with RC-3 notice the icon lines up with the first line of text: 
And with this fix now the icon is above the first line of text: 
Any ideas? Please let me know if more testing or info helps. |
|
I've retested with the updated PR and while the multi-line issues looks to be addressed a new issue has appeared where the space between bold and non-bold text is removed: 
You can see this live at https://docsify-preview-61gtha3je-docsifyjs.vercel.app/preview/#/ui-kit?id=callouts I also retested with the original codebox and can confirm that the multline text now aligns with the callout icon as expected: Thanks very much, Paul. |
paulhibbitts
left a comment
paulhibbitts
left a comment
There was a problem hiding this comment.
Thanks @sy-records , this looks good with both the UIKit page and my additional tests👍🏼
sy-records
changed the title
* develop: (126 commits) refactor: improve blockquoteCompiler logic (#2624) docs: add DeployHQ deployment instructions (#2627) docs: fix broken link to issues in CONTRIBUTING.md (#2625) chore: bump rimraf from 5.0.7 to 6.1.0 (#2619) chore: bump @rollup/plugin-commonjs from 28.0.1 to 29.0.0 (#2618) chore: bump actions/setup-node from 5 to 6 (#2620) chore: bump actions/upload-artifact from 4 to 5 (#2621) chore: bump stefanzweifel/git-auto-commit-action from 6 to 7 (#2622) test: improve file embed & code fragment tests (#2617) test: add test for file embed and code fragments (#2616) feat: add fallback default language support (#2607) chore: bump actions/setup-node from 3 to 5 (#2609) chore: bump actions/checkout from 4 to 5 (#2608) docs: Update intro paragraph to help improve first impressions and broaden audience (#2602) feat: enhance embed handling for table cells (#2606) fix: enhance accessibility for sidebar toggle button (#2604) [release] 5.0.0-rc.3 [release] 5.0.0-rc.2 fix: normalize slugs to NFC and remove emoji variation selector (#2597) feat: GitHub style callouts (#2487) ...
Summary
Related issue, if any:
Fix #2623
What kind of change does this PR introduce?
For any code change,
Does this PR introduce a breaking change?
Tested in the following browsers: