Skip to content

(feat) O3-3498 : Define tutorial extension for the help menu #2

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

Merged
merged 2 commits into from
Jul 3, 2024

Conversation

Vijaykv5
Copy link
Member

@Vijaykv5 Vijaykv5 commented Jun 27, 2024

Requirements

  • This PR has a title that briefly describes the work done including a conventional commit type prefix and a Jira ticket number if applicable. See existing PR titles for inspiration.

For changes to apps

If applicable

  • My work includes tests or is validated by existing tests.

Summary

This PR creates an extension for Tutorial component in the help-menu-slot

Screenshots

tutorial-extension.mov

Related Issue

O3-3498

Other

Copy link
Collaborator

@Piumal1999 Piumal1999 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

I have two suggestions:

  1. It would be better to rename the src/tutorial.tutorial.tsx component to something that clearly reflects its usage as a help-menu-item pushed to the help-menu-slot extension slot. I don't have a suitable name in mind at the moment. @jayasanka-sack, what do you think?

  2. I noticed that the stylings related to the help menu list item have been rewritten here, which is redundant. It would be better to keep the stylings within the esm-help-menu-app, so the extension will contain only the text and the internal functionalities.

@Vijaykv5
Copy link
Member Author

Vijaykv5 commented Jul 1, 2024

@Piumal1999
For 2. I've made styling to the extension-slot here, so it wouldn't ruin the styling of newly added extension and can be kept under esm-help-menu-app

Copy link
Member

@jayasanka-sack jayasanka-sack left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @Vijaykv5 !

@jayasanka-sack jayasanka-sack merged commit 9cca91f into openmrs:main Jul 3, 2024
4 of 5 checks passed
@Vijaykv5 Vijaykv5 deleted the new branch July 5, 2024 12:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants