-
-
Notifications
You must be signed in to change notification settings - Fork 2k
fix(www): API Pages Responsive #4821
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: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for ngrx-io canceled.Built without sensitive environment variables
|
✅ Deploy Preview for ngrx-site-v19 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
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.
This looks good to me.
I only notice a double scroll bar on a normal screen (this is also the case before these changes).
https://ngrx-site-v19.netlify.app/api/store/createActionGroup
We can create a new issue for this if it isn't in scope of this issue.
Thanks @timdeschryver for the review and approval. Maybe that double scroll bar should be in new issue like you suggest. |
overflow-x: auto; | ||
overflow-x: 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.
This was the fix in the previous PR to remove duplicate horizontal scroll bars on the page. We can revert it to overflow-x: auto
.
@media only screen and (max-width: 1280px) { | ||
overflow-x: hidden; | ||
} |
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.
This can be removed. It's better to show the scrollbar if needed instead of hiding it and disabling scroll functionality. If the scrollbar is unnecessarily displayed, we should identify which element causes the issue and fix its width.
@media only screen and (max-width: 1280px) { | ||
white-space: pre-wrap; | ||
} |
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.
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Api pages are not responsive
Closes #4816
What is the new behavior?
Api pages are responsive
Does this PR introduce a breaking change?