Skip to content

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

ashok-2003
Copy link
Contributor

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

  • Yes, I signed my commits.

Signed-off-by: Ashok Kumar <ashokgupta2678@gmail.com>
@ashok-2003
Copy link
Contributor Author

here we go @vr-varad , please review the changes.

@l5io
Copy link
Contributor

l5io commented May 27, 2025

🚀 Preview for commit e3137b3 at: https://68357666c3c24a919a437dff--layer5.netlify.app

@ashok-2003
Copy link
Contributor Author

Umm, the single line looks kind of bad without a scrollbar. Is this the expected behavior, or am I missing something?
Screenshot 2025-05-27 163419

@vr-varad
Copy link
Contributor

hey @ashok-2003 any update on the pr?

@ashok-2003
Copy link
Contributor Author

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!
Also, I’ve attached an image — is this the kind of result we’re aiming for? If so, we might just need to adjust the z-index to fix the remaining part. A quick review would help — I’ll push the PR based on your feedback.
Screenshot 2025-05-27 163419
is it should be single line like this ? rest i will adjust based on feedback.

@vr-varad
Copy link
Contributor

nope!! the earlier was better @ashok-2003

@vr-varad vr-varad requested review from vishalvivekm and M-DEV-1 May 29, 2025 15:03
@ashok-2003
Copy link
Contributor Author

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?

@vr-varad
Copy link
Contributor

lets get others' review on multi line one and make changes accordingly

@vr-varad vr-varad closed this May 29, 2025
@vr-varad vr-varad reopened this May 29, 2025
@l5io
Copy link
Contributor

l5io commented May 29, 2025

🚀 Preview for commit e3137b3 at: https://68387cc5083d3e04e4641cfa--layer5.netlify.app

@M-DEV-1
Copy link
Member

M-DEV-1 commented May 30, 2025

image
This would be the ideal case, for the codeblock, with some minor fixes to the position of the copy button.

There's a video with the expected changes in the issue description @ashok-2003

@ashok-2003
Copy link
Contributor Author

Okay @M-DEV-1, I’ll try to implement that.
But my main concern is how it should behave on mobile screens.
If we go with a single line layout without a scrollbar, it might overflow the screen.

@vr-varad
Copy link
Contributor

add the scroll below a curtain screen width when the text starts overflow.
Hope you understand,

Signed-off-by: Ashok Kumar <ashokgupta2678@gmail.com>
@l5io
Copy link
Contributor

l5io commented May 30, 2025

🚀 Preview for commit c2548f7 at: https://683a19b15bf7e264e8b327d1--layer5.netlify.app

@ashok-2003
Copy link
Contributor Author

ashok-2003 commented May 31, 2025

@vr-varad , @M-DEV-1, looking forward for review !
Screenshot 2025-05-30 211103

@M-DEV-1
Copy link
Member

M-DEV-1 commented May 31, 2025

hi @ashok-2003, LGTM on desktop view but you should also take a look on how it's renderedn in mobile view.

@ashok-2003
Copy link
Contributor Author

Hey @M-DEV-1, I’ve also checked for smaller screens. I implemented the following media query:

@media (max-width: 1024px) {
display: block;
overflow-x: auto;
}
This ensures that below that screen width, a scrollbar appears so the content doesn’t overflow.
Screenshot 2025-05-31 165340

@vr-varad
Copy link
Contributor

vr-varad commented Jun 2, 2025

dont use media query use theme breakpoints. @ashok-2003

@ashok-2003
Copy link
Contributor Author

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.
Screenshot 2025-06-04 051316

Screenshot 2025-06-04 051248

Signed-off-by: Ashok Kumar <ashokgupta2678@gmail.com>
@l5io
Copy link
Contributor

l5io commented Jun 4, 2025

🚀 Preview for commit 13518c4 at: https://683f8cf88154ee450d7eb553--layer5.netlify.app

@LibenHailu
Copy link
Contributor

LGTM!

@ashok-2003
Copy link
Contributor Author

Hey @vr-varad, @M-DEV-1, I think it's working perfectly fine. Are we good to go on this one now?
please review at
https://683f8cf88154ee450d7eb553--layer5.netlify.app/cloud-native-management/meshery/getting-started

@M-DEV-1
Copy link
Member

M-DEV-1 commented Jun 4, 2025

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?

@ashok-2003
Copy link
Contributor Author

@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>
@ashok-2003
Copy link
Contributor Author

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

@l5io
Copy link
Contributor

l5io commented Jun 11, 2025

🚀 Preview for commit 12fbb92 at: https://6849b4178f84ef3e66119055--layer5.netlify.app

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ensure Full Command Visibility in Installation Terminal
5 participants