Skip to content

Conversation

dchyun
Copy link
Contributor

@dchyun dchyun commented May 6, 2025

📌 Summary

Preview

If merged, this PR would fix rendering issues in the CodeBlock with the line wrapping and line highlighting. The primary issues found and resolved are.

  • Line numbers are not displayed correctly when line wrapping is enabled
  • Line numbers are not displayed correctly when the number of lines is updated dynamically
  • Line numbers and line highlighting are not displayed correctly when line wrapping is enabled, and the code block is hidden from view initially (ex: inside a tab)
  • Line highlighting is not displayed correctly when line numbers are not present

The CodeBlock was also converted from using did-insert and did-update to custom modifiers.

🛠️ Detailed description

Originally in #2826 there were several issues observed in the CodeBlock. It was observed that when the code block was used inside the tabs the line numbers and line highlighting of the code blocks inside tabs initially hidden were not rendering correctly.

After additional investigation several issues with the Prism.js library were contributing to this problem.

Line numbers issues when line wrapping is enabled

First, line numbers are not being rendered correctly in the following situation

  • hasLineWrapping is true
  • no lines are highlighted
  • there is only one CodeBlock on the page

The reason this issue has not been caught before is that when line highlighting is true, we trigger a resize event on the window to re-trigger Prism.js's line highlighting rendering. When there are other code blocks on a page that have line highlighting, this event was being listened to by code blocks without line highlighting as well. It would then re-trigger a Prism update, which would fix the line number issues.

If a page has no other code blocks with line highlighting to trigger this resize event, then the issues with the line numbers are not fixed.

Line number and highlight issues inside tabs

When the code block is initially hidden, such as being inside an inactive tab, the line numbers and highlighting are not rendered correctly when it is made visible, if hasLineWrapping is true.

When the code block is hidden on load, the resize event to re-trigger the prism.js updates does not correctly apply the styles to the hidden content. To solve this issue, we are now re-triggering Prism.js when the code block becomes visible using an IntersectionObserver.

Line highlighting with no line numbers

When line highlighting is set but hasLineNumbers is false the highlights start one line earlier than they should. This is due to Prism js calculating the line numbers based on the pre element when line numbers are present, but calculating it based on the code element when they are not (source code link).

📸 Screenshots

Line number & wrapping issues

Note: This was only possible to show locally by only displaying one code block on the showcase page

Before
Screenshot 2025-05-05 at 6 50 51 PM

After
Screenshot 2025-05-05 at 6 50 22 PM

Tabs issue

Before
Screenshot 2025-05-06 at 8 20 11 AM

After
Screenshot 2025-05-06 at 8 27 27 AM

Line highlighting with no line numbers

Before
Screenshot 2025-05-16 at 11 00 29 AM

After
Screenshot 2025-05-16 at 11 00 38 AM

🔗 External links

Jira ticket: HDS-4875


👀 Component checklist

💬 Please consider using conventional comments when reviewing this PR.

Copy link

vercel bot commented May 6, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
hds-showcase ✅ Ready (Inspect) Visit Preview May 16, 2025 4:03pm
hds-website ✅ Ready (Inspect) Visit Preview May 16, 2025 4:03pm

// we need to re-trigger the line numbers generation as late as possible to account for any line wrapping styles that are applied
if (this.args.hasLineWrapping && Prism?.plugins?.['lineNumbers']) {
// eslint-disable-next-line @typescript-eslint/no-unsafe-call, @typescript-eslint/no-unsafe-member-access
Prism.plugins['lineNumbers'].resize(this._preCodeElement);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can't use the same logic as the line highlight and piggy back on triggering a resize event because the Prism.js line number plugin does a check to see if the viewport width actually changes on a resize event. It aborts any updates if the viewport is unchanged.

@dchyun dchyun force-pushed the dchyun/code-block-tabs-render-fix branch from 845b2d3 to 6fe1258 Compare May 6, 2025 13:54
@dchyun dchyun marked this pull request as ready for review May 6, 2025 14:14
@dchyun dchyun requested a review from a team as a code owner May 6, 2025 14:14
Base automatically changed from hds-4745-codeblock-height-toggle to main May 13, 2025 12:48
Copy link
Contributor

@didoo didoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dchyun I did a quick first pass, but happy to do a second more in depth pass if/when you want

didoo
didoo previously approved these changes May 16, 2025
Copy link
Contributor

@didoo didoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a couple of comments/suggestions but nothing that is a blocker.

@dchyun
Copy link
Contributor Author

dchyun commented May 16, 2025

Caught a new issue in prod with line highlighting when hasLineNumbers is false. Issue has been fixed and details are in the description.

@dchyun dchyun requested review from didoo and KristinLBradley May 16, 2025 15:03
Copy link
Contributor

@didoo didoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great PR! 👏👏👏

@dchyun dchyun merged commit 76d8575 into main May 16, 2025
16 checks passed
@dchyun dchyun deleted the dchyun/code-block-tabs-render-fix branch May 16, 2025 20:24
@hashibot-hds hashibot-hds mentioned this pull request May 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants