-
Notifications
You must be signed in to change notification settings - Fork 91
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
Add DocsExample #873
Add DocsExample #873
Conversation
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.
Thanks @chetan21122004 and @ozer550, wonderful work so far. Posting few in-code comments, mostly last touches. Then I think we could be ready for merge.
Hi @MisRob, Thank you for your detailed feedback. As a beginner, I found this task challenging and apologize for any inconvenience caused, including the unwanted changes I made as I didn’t use a formatter. I truly appreciate your patience and guidance. I will review your commits and make sure my setup avoids unrelated updates. I’m working on improving my coding practices and will run yarn lint-fix to ensure clean contributions. Your support means a lot, and I look forward to learning more from you. Thank you again! |
No problem @chetan21122004, I think lots of it was actually caused by the Node version transition. Let's see how it goes, now we have a clean state. Let us know if something unexpected would happen with linting. |
Hi @chetan21122004! Regarding the change log, yes it should not be present in the PR. Rather we need to fill in the changelog in the pull request template. For example here. |
Hi @chetan21122004, just checking in - would you need anything from us here? |
Hi @MisRob , Thanks for asking but now I didn't need anything , I will let you know if I have any issues, Thanks for your support, And few days I can't able to be active because my mid term examination is going on sorry for this..! |
Great @chetan21122004, no pressure, I'm just checking in on PRs occasionally. This task has no deadline, please take all the time you need. Good luck with exams :)! |
Hi @chetan21122004, we have some new dependencies on this pull request in #905 so I will go ahead and push the remaining cleanup here so we can merge. Also in #905, we discussed some changes to the You're very welcome to keep an eye on 'help wanted' issues - over time there will be new ones regarding this component. I'm sorry for not waiting longer, this was the best solution I could think of to make work in those other areas manageable. Thanks a lot for this work, it has laid down very nice foundation! |
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.
Thanks @chetan21122004 and @ozer550! I will merge now so we can proceed with work that depends on this component mentioned earlier.
Description:
I’ve added the DocsExample component that allows toggling between an example and its code.
Issue Addressed:
Fixes #826. Also references #836 for additional context on previous discussions related to the component.
Guidance:
The new component enhances the documentation UI by toggling code visibility.
Steps to Test:
Implementation Notes:
Changelog
DocsExample
component with toggle functionality for switching between an example and its code snippet in the KDS documentation.DocsExample
that allows to toggle between an example and its code #826