-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
base: master
Are you sure you want to change the base?
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 |
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