-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
fix: rest api response box is unreachable via scroll #29830
fix: rest api response box is unreachable via scroll #29830
Conversation
WalkthroughThe code update introduces a CSS modification aimed at improving the user experience in scrolling through long REST API responses. By adjusting the overflow and padding settings for the Changes
Assessment against linked issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
/build-deploy-preview skip-tests=true |
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/7315678477. |
Deploy-Preview-URL: https://ce-29830.dp.appsmith.com |
/build-deploy-preview skip-tests=true |
2 similar comments
/build-deploy-preview skip-tests=true |
/build-deploy-preview skip-tests=true |
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/7327104040. |
@vaibhavchobisa looks like the issue is still there. You can sign up with a new user and test here: https://ce-29830.dp.appsmith.com/ Screen.Recording.2023-12-26.at.11.35.03.mov |
Deploy-Preview-URL: https://ce-29830.dp.appsmith.com |
Hey @rohan-arthur |
tested ok on incognito. |
@@ -13,6 +13,12 @@ const TabPanelWrapper = styled(TabPanel)` | |||
&.ads-v2-tabs__panel { | |||
overflow: auto; | |||
} | |||
& .t--code-editor-wrapper.codeWrapper { | |||
overflow-y: scroll; |
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.
@vaibhavchobisa Why is this overflow-y necessary? It seems to be adding another scroll bar in the UI, other than the one we already have for code editor
& .t--code-editor-wrapper.codeWrapper { | ||
overflow-y: scroll; | ||
& .CodeMirror-scroll { | ||
padding-bottom: 0; |
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.
The root cause of the issue is that the div with class CodeMirror-scroll does not have its height configured correctly, its height is overflowing over the footer and hence some of the content gets hidden behind the footer, can you please check where this height is coming from and fix the issue from root cause?
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.
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.
Thank you for the feedback @sneha122.
Div with the class "t--code-editor-wrapper codeWrapper" is the one that is causing the overflow beyond the screen. I have provided it with the height of "100% - 40px" which takes both the footer size and the little padding above it into account.
I have also replaced the bottom padding with box-sizing: border-box; so that the bottom padding of 50px stays.
It seems to be working fine now, please re-review and let me know, thanks!
/build-deploy-preview skip-tests=true |
This PR has not seen activitiy for a while. It will be closed in 7 days unless further activity is detected. |
Hi @sneha122 |
/build-deploy-preview skip-tests=true |
/ok-to-test tags="@tag.All" |
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/7406261278. |
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/7406261868. |
Deploy-Preview-URL: https://ce-29830.dp.appsmith.com |
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/7406261868. |
This PR has not seen activitiy for a while. It will be closed in 7 days unless further activity is detected. |
This PR is active |
This PR has not seen activitiy for a while. It will be closed in 7 days unless further activity is detected. |
This PR has been closed because of inactivity. |
This PR has been closed because of inactivity. |
@vaibhavchobisa Apologies for the delay here, but since this PR has become very old and our automation processes have been updated since then, I would kindly request you to sync the forked repo with the original one and merge latest release into the branch, this way everything will be updated and we can proceed ahead with the PR. |
…eachable-via-scrollbar
@sneha122 Done. Thanks! |
/build-deploy-preview skip-tests=true |
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/8598157520. |
Deploy-Preview-URL: https://ce-29830.dp.appsmith.com |
All automation test cases have passed in this [draft PR created internally] Also tested the PR for the issue fix and it looks as expected |
Description
Scrollbar of API response is escaping the viewport. This PR fixes the issue.
Media
Fixes #29534
or
Fixes
Issue URL
Warning
If no issue exists, please create an issue first, and check with the maintainers if the issue is valid.
Automation
/ok-to-test tags="@tag.Datasource"
🔍 Cypress test results
Caution
If you modify the content in this section, you are likely to disrupt the CI result for your PR.
Summary by CodeRabbit
TabPanelWrapper
component for better readability and user interface experience.DebuggerLogs
component to enhance the user interface.