-
Notifications
You must be signed in to change notification settings - Fork 3
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
feat: update header for sidebar #69
Conversation
b8e6dfb
to
ad8de19
Compare
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.
lgtm!
@@ -116,7 +111,7 @@ const Sidebar = ({ | |||
alt="close" | |||
aria-label="close" | |||
variant="primary" | |||
invertColors={!disclosureAcknowledged} | |||
invertColors |
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.
Nice!
@@ -36,7 +36,7 @@ const courseId = 'course-v1:edX+DemoX+Demo_Course'; | |||
const unitId = 'block-v1:edX+DemoX+Demo_Course+type@unit+block@unit1'; | |||
|
|||
const assertSidebarElementsNotInDOM = () => { | |||
expect(screen.queryByTestId('heading', { name: 'Hi, I\'m Xpert!' })).not.toBeInTheDocument(); | |||
expect(screen.queryByTestId('sidebar-xpert-header')).not.toBeInTheDocument(); |
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.
Nice! I like that you made the test id more descriptive.
COSMO-571
This pull request includes changes to the
Sidebar
component to update its styling and improve its functionality. The most important changes include adding a new header with a logo, modifying the separator styling, and updating the corresponding tests.Old sidebar header:
New sidebar h
eader: