-
Notifications
You must be signed in to change notification settings - Fork 7
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) O3-3890 Help menu (and menu items) should use a standard Carbon… #21
base: main
Are you sure you want to change the base?
Conversation
src/tutorial/tutorial.test.tsx
Outdated
render(<Tutorial />); | ||
|
||
const menuItem = screen.getByText(/Tutorials/i); | ||
fireEvent.click(menuItem); |
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.
Prefer userEvent instead.
src/tutorial/tutorial.test.tsx
Outdated
jest.mock('@openmrs/esm-framework', () => ({ | ||
showModal: jest.fn(), | ||
})) |
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.
jest.mock('@openmrs/esm-framework', () => ({ | |
showModal: jest.fn(), | |
})) | |
const mockShowModal = jest.mocked('showModal'); |
And then replace showModal
in assertions with mockShowModal
. See https://o3-docs.openmrs.org/docs/frontend-modules/unit-and-integration-testing#you-should-not-need-to-use-partial-mocks for more context about why you shouldn't need to use partial mocks.
src/tutorial/styles.scss
Outdated
} | ||
|
||
.tutorialsText:hover { | ||
background: #e8e8e8; |
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.
background: #e8e8e8; | |
background: colors.$white-hover; |
src/tutorial/styles.scss
Outdated
@@ -28,3 +28,11 @@ | |||
|
|||
} | |||
} | |||
|
|||
.tutorialsText { | |||
padding: 0.5rem 0.5rem; |
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.
nit:
padding: 0.5rem 0.5rem; | |
padding: layout.$spacing-03; |
@Samstar10 May you please update this PR as per the changes requested.. |
Done @Vijaykv5 |
… menu
Requirements
For changes to apps
If applicable
Summary
I have updated the help menu to use standard carbon menu items. This PR is related to this PR, openmrs/openmrs-esm-core#1164.
Screenshots
Please find attached the screenshot.
Related Issue
https://openmrs.atlassian.net/browse/O3-3890
Other