-
Notifications
You must be signed in to change notification settings - Fork 1
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
OV-75: add sidebar and header to home page #83
OV-75: add sidebar and header to home page #83
Conversation
Please change your target branch to task/OV-13-add-side-bar-component, so that we can see a valid diff. |
I changed branch to task/OV-62 because the PR from branch 13 was closed and inthe issue i says the new branch is 62. https://github.com/orgs/BinaryStudioAcademy/projects/30/views/1?pane=issue&itemId=75054501 |
When there are new notifications, the bell icon should also have little orange circle (as in design) |
In Header task the bell icon is crossed. Maybe we should not implement it now? |
This reverts commit c03b5c1.
Box, | ||
Header, | ||
Sidebar, | ||
} from '~/bundles/common/components/components.js'; | ||
|
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.
You just need to add the overflow: auto; property to the main wrapper.
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.
I think you should remove left padding page, in order to see the sidebar items more centered
Which left padding? |
The changes that you need are already in next, so you can change your base branch to next, because right now we cannot see valid diff. |
The base branch was changed.
@@ -16,8 +16,7 @@ const Header: React.FC<Properties> = ({ left, center, right }) => { | |||
left="0" | |||
width="full" | |||
backgroundColor="background.900" | |||
boxShadow="md" | |||
zIndex="100" | |||
zIndex="1000" |
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.
Does zIndex="100" not work?
Because in this pr we changed it from 1000 to 100 :)
474336d
Created branch from branch OV-62.
Proposed some changes of Sidebar in pull request #66 that could benefit this task