-
Notifications
You must be signed in to change notification settings - Fork 1.4k
fix codeblock horizontal-scroll issue #6494
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
Conversation
Signed-off-by: Ashok Kumar <ashokgupta2678@gmail.com>
here we go @vr-varad , please review the changes. |
🚀 Preview for commit e3137b3 at: https://68357666c3c24a919a437dff--layer5.netlify.app |
hey @ashok-2003 any update on the pr? |
Hey @vr-varad, sorry I couldn’t look into the issue today — I have my end-sem exam scheduled for tomorrow. I’ll be able to tackle it after that. Hope you understand! |
nope!! the earlier was better @ashok-2003 |
Hey @vr-varad, I’m a bit confused — could you point out the image or reference we should proceed with? Are you referring to the multi-line version or the single-line one? |
lets get others' review on multi line one and make changes accordingly |
🚀 Preview for commit e3137b3 at: https://68387cc5083d3e04e4641cfa--layer5.netlify.app |
There's a video with the expected changes in the issue description @ashok-2003 |
Okay @M-DEV-1, I’ll try to implement that. |
add the scroll below a curtain screen width when the text starts overflow. |
Signed-off-by: Ashok Kumar <ashokgupta2678@gmail.com>
🚀 Preview for commit c2548f7 at: https://683a19b15bf7e264e8b327d1--layer5.netlify.app |
hi @ashok-2003, LGTM on desktop view but you should also take a look on how it's renderedn in mobile view. |
dont use media query use theme breakpoints. @ashok-2003 |
Hey @vr-varad, I went deeper into the codebase and used the AnimatedStepsListWrapper breakpoint. Unfortunately, I couldn’t find any special theme breakpoints. Here's the code for both AnimatedStepsListWrapper and MesheryWrapper styles—I noticed there’s some inconsistency in their media queries as well. Since I couldn’t find any custom theme breakpoints, I’ve temporarily used the same breakpoints as in AnimatedStepsListWrapper to maintain uniformity. Hope that works for now. Let me know if you find anything different. |
Signed-off-by: Ashok Kumar <ashokgupta2678@gmail.com>
🚀 Preview for commit 13518c4 at: https://683f8cf88154ee450d7eb553--layer5.netlify.app |
LGTM! |
Hey @vr-varad, @M-DEV-1, I think it's working perfectly fine. Are we good to go on this one now? |
Looks good, but it has more vertical padding than necessary? I think it would be nice to have some horizontal padding, provided that it doesn't break. @ashok-2003 @vr-varad thoughts? |
@M-DEV-1 , extra vertical padding coming from the pre div render inside the pre div. May be extra horizontal padding might make it look more chunkier. might not be good idea(According to me ). or Might be ! |
Signed-off-by: Ashok Kumar <ashokgupta2678@gmail.com>
Thanks @M-DEV-1. As discussed in the meeting, I’ve undone the changes made directly to the code block and ensured that the updates remain limited to the Hero section only. Please review! here is the video - Screen.Recording.2025-06-11.113949.mp4 |
🚀 Preview for commit 12fbb92 at: https://6849b4178f84ef3e66119055--layer5.netlify.app |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
Hi @ashok-2003, looks good to me. Thanks for restricting the changes to just that specific page component. |
🚀 Preview for commit a458397 at: https://685e5f0d87e4de40ab1bcebb--layer5.netlify.app |
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.
@ashok-2003 it's not about the code block in hero section only.
In installing section, down the page, where we need to have similar consistent behavior.
Thanks a lot for working on this @ashok-2003, incorporating multiple rounds of feedback.
Again don't think, this is an valid issue which needs fixed necessarily. |
Description
This PR fixes #6488
Notes for Reviewers
Added :
white-space: pre-wrap; - to ensure text-wrap to words
word-break: break-word; - to ensure that long URL will break and wrap
Screen.Recording.2025-05-27.014112.mp4
Signed commits