Issue 921
Description
Added a Usage section to the KModal documentation.
Issue addressed
Before/after screenshots
Before
After
Changelog
- Description: Added a Usage section to the KModal documentation. Usage section items 1. Dialog Box with Description 2. Submit Form Dialog Box 3. Modal with Different Sizes
- Products impact: None
- Addresses: #921
- Components: - None
- Breaking: No
- Impacts a11y: No
- Guidance: None
Steps to test
- Step 1
- Step 2
- ...
(optional) Implementation notes
At a high level, how did you implement this?
Does this introduce any tech-debt items?
Testing checklist
- [ ] Contributor has fully tested the PR manually
- [ ] If there are any front-end changes, before/after screenshots are included
- [ ] Critical and brittle code paths are covered by unit tests
- [ ] The change is described in the changelog section above
Reviewer guidance
- [ ] Is the code clean and well-commented?
- [ ] Are there tests for this change?
- [ ] Are all UI components LTR and RTL compliant (if applicable)?
- [ ] Add other things to check for here
Comments
It has a dependency on the PR below, so we need to merge that one first. PR: https://github.com/learningequality/kolibri-design-system/pull/1021
Hi @yeshwanth235, thanks for the contribution! This will be helpful.
- There are some unrelated files, such as
kcardandDocsExample. Seems it just leaked from another task? In this branch, you will need to keep only changes related to the documentation page you intend to update. - In the screenshot, I see titles of sections are capitalized. Please use "Sentence case" capitalization for titles, not "Title Case" capitalization, and on that opportunity, I'd like to ask you to have a look these Writing guidelines. After you feel it is aligned, let me know and I will review in detail.
Hi @MisRob, I agree with the first point. I am dependent on [#1021]. Once it is merge. Will Update the PR.
@yeshwanth235 You can remove chosen files from a pull request (= revert to develop versions) with git checkout develop -- <file_path> and then commit. That said, this is not a pressing task, and no problem to wait. I'm suggesting this in case you didn't know about this command and perhaps want to learn something new.
And for next time, if you pick up another issue, it is possible and common to open multiple pull requests at once for various tasks - by starting a new branch from develop for each task rather than from your another working branch. And if this happens by mistake, you can look into rebasing.
To make it clear. Since i needed hideStyle prop. I have created my branch from existing issue. Once we merge that PR. I will take pull from develop. It will resolve the issue and only show issue 921 changes. I hope you get it 🙂 @MisRob
Thanks for clarification, I assume you mean this place:
<DocsExample
loadExample="KModal/ModalWithDifferentSizes.vue"
:hideStyle="true"
/>
I haven't noticed before. Yes then, it's best to wait :+1:
Hi @MisRob Updated the PR. Please help to review
Percy Visual Test Results
Percy Dashboard: View Detailed Report
Environment:
- Node.js Version: 18.x
- OS: Ubuntu-latest
Instructions for Reviewers:
- Click on the Percy Dashboard link to view detailed visual diffs.
- Review the visual changes highlighted in the report.
- Approve or request changes based on the visual differences.
Great, I will have a look @yeshwanth235
It's possible that those previous unresolved comments were hidden under 'Load more':