Skip to content

Conversation

KristinLBradley
Copy link
Contributor

@KristinLBradley KristinLBradley commented Apr 16, 2025

📌 Summary

If merged, this PR adds a height toggle control feature for expanding the height of the CodeBlock which appears when a maxHeight value is set that exceeds by the content height.

Showcase: https://hds-showcase-git-hds-4745-codeblock-height-toggle-hashicorp.vercel.app/components/code-block#height-limit

📸 Screenshots

Before
Screenshot 2025-05-01 at 4 39 15 PM

After
Screenshot 2025-05-01 at 4 38 53 PM

🔗 External links


👀 Component checklist

💬 Please consider using conventional comments when reviewing this PR.

Copy link

vercel bot commented Apr 16, 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 12, 2025 3:31pm
hds-website ✅ Ready (Inspect) Visit Preview May 12, 2025 3:31pm

@KristinLBradley

This comment was marked as outdated.

@majedelass

This comment was marked as duplicate.

@shleewhite
Copy link
Contributor

So Dylan and I are starting to uncover a few issues related to the gradient overlay used with the overflowing code content...

If you scroll horizontally, the gradient no longer covers the sides of the code content. If you try to compensate by stretching the width of the gradient overlay to be wider that 100% then it introduces unnecessary horizontal scroll bars when not needed.

These issues are related to styles adding padding around the code content. Restructuring this padding can fix the positioning issues with the gradient overlay but it then throws off how the the JS calculates positioning for line highlighting.

I'm not sure of the best way forward to solve these issues.

We could possibly decide that we don't mind the unnecessary horizontal scrollbar or else decide that we don't mind that the gradient doesn't cover the full width of the code in the case the user scrolls horizontally when needed.

Or else we could come up with a different design solution such as Andrew's original design which added a bottom footer to contain the height toggle button vs. having the button overlay the code content. Or else we could find another way to increase contrast with the overlaid button and code content such as by using a dropshadow behind the button or something else instead of the current gradient.

Issue 1: Unnecessary horizontal scrollbar (if gradient is stretched to fix issue #2) image

Issue 2: Gradient doesn't cover both sides if user scrolls horizontally image

@KristinLBradley Not to throw another option in the ring - but the way the advanced table shadows work is that they are a sibling of the grid and then the height/width are set appropriately and its positioned absolutely on top of the grid. that way if you scroll it always looks correct.

@dchyun dchyun removed this from the [email protected] milestone May 8, 2025
@dchyun
Copy link
Contributor

dchyun commented May 8, 2025

@KristinLBradley Not to throw another option in the ring - but the way the advanced table shadows work is that they are a sibling of the grid and then the height/width are set appropriately and its positioned absolutely on top of the grid. that way if you scroll it always looks correct.

@shleewhite The issue is that the gradient needs to be a child of the overflowed container, and have position: sticky. This is so it can avoid overlapping the line numbers, but move with the content as a user scrolls horizontally.

We tried placing the gradient as a sibling, and using z-index to have the line numbers appear above, but then part of the code is exposed when scrolling horizontally.

Screenshot 2025-05-08 at 11 40 44 AM

@KristinLBradley

This comment was marked as resolved.

@dchyun
Copy link
Contributor

dchyun commented May 9, 2025

@dchyun Just noticed an issue with line-wrapping not working properly:

@KristinLBradley thanks for catching that. I added the display: block when line wrapping is enabled.

@KristinLBradley

This comment was marked as resolved.

@dchyun
Copy link
Contributor

dchyun commented May 9, 2025

I just checked Safari as well though, and I am seeing an inconsistent glitch happen in which there is a lot of extra space at the bottom AND the line doesn't stretch. It doesn't happen consistently though.
So far not seeing this glitch in either Firefox or Edge...

This issue seems to not be replicable on anyone's instances of Safari, but to those reviewing please test just to double check no one else can replicate this.

@dchyun dchyun requested a review from andgen404 May 9, 2025 18:17
shleewhite
shleewhite previously approved these changes May 12, 2025
Copy link
Contributor

@shleewhite shleewhite left a comment

Choose a reason for hiding this comment

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

Tested more and it seems like the gradient is working well now!

Copy link
Contributor

@andgen404 andgen404 left a comment

Choose a reason for hiding this comment

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

🎉

@dchyun dchyun merged commit 130b668 into main May 13, 2025
16 checks passed
@dchyun dchyun deleted the hds-4745-codeblock-height-toggle branch May 13, 2025 12:48
@hashibot-hds hashibot-hds mentioned this pull request May 13, 2025
@alex-ju alex-ju added this to the [email protected] milestone May 13, 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.

8 participants