(fix) O3-3896: Tutorials modal should use a standard Carbon modal component #20
Conversation
|
@chibongho @vasharma05 @usamaidrsk could you please review this PR, your feedback will be appreciated. |
Update:
|
|
Thanks @IamMujuziMoses. Looks fine to me, but I'm not too familiar with this code base and don't have permission to approve PRs in this repo. @jayasanka-sack @Vijaykv5 Can you take a look? |
src/tutorial/modal.component.tsx
Outdated
| <p className={styles.description}> | ||
| {t('modalDescription', 'Find walkthroughs and video tutorials on some of the core features of OpenMRS.')} | ||
| </p> | ||
| </ModalHeader> | ||
| <ModalBody className={styles.tutorialModal}> | ||
| <ModalBody className={styles.tutorialModal} hasScrollingContent > |
There was a problem hiding this comment.
Can we remove this? by default wrapping up over <React.Fragment> make it scrollable.
You can read it here #docs
| <ModalBody className={styles.tutorialModal} hasScrollingContent > | |
| <ModalBody className={styles.tutorialModal} > |
| <ul> | ||
| {tutorials.map((tutorial, index) => ( | ||
| <li className={styles.tutorialItem} key={index}> | ||
| <h3 className={styles.tutorialTitle}>{tutorial.title}</h3> | ||
| <p className={styles.tutorialDescription}>{tutorial.description}</p> | ||
| <Button kind="ghost" onClick={() => handleWalkthroughClick(index)}> | ||
| <Link className={styles.tutorialLink} onClick={() => handleWalkthroughClick(index)} renderIcon={() => | ||
| <ArrowRight aria-label="Arrow Right" />}> |
There was a problem hiding this comment.
@jayasanka-sack do we need to keep an Arrow Right here?
It's not defined in the design though.
@IamMujuziMoses Also please make the cursor to pointer
There was a problem hiding this comment.
@Vijaykv5 it looks okay to me. Let's keep it.
+1 to cursor
There was a problem hiding this comment.
I think the cursor is set to pointer by default for the link component. Please confirm.
IamMujuziMoses
force-pushed
the
O3-3896
branch
2 times, most recently
from
23baf38 to
f72c2e9
Compare
|
The E2E test is failing when trying to click the walkthrough button. Could you please fix it? |
IamMujuziMoses
force-pushed
the
O3-3896
branch
from
f72c2e9 to
5f93acc
Compare
There was a problem hiding this comment.
Please fix the failing E2E test as mentioned in here: #20 (comment)
Requirements
For changes to apps
If applicable
Summary
This PR implements a fix for the Tutorial Modal styling of the Carbon Modal component.
Screenshots
Before
Screen.Recording.2024-09-24.at.21.06.26.mov
After
Screen.Recording.2024-09-24.at.21.07.11.mov
Related Issue
https://issues.openmrs.org/browse/O3-3896