kolibri-design-system
kolibri-design-system copied to clipboard
[Visual testing]: Add visual tests for KCheckbox
🌱 Are you new to the codebase? Welcome! Please see the contributing guidelines.
Context
We recently introduced a new visual testing system in KDS to prevent visual regressions and ensure UI consistency, and we want to start populating our KDS components with visual tests.
If you are not familiar yet with our visual testing system. Please refer to our Visual Testing Guide. You can also check the existing tests for KButton as an example.
Additionally, see the KCheckbox documentation for reference.
Desired behavior
KCheckbox should have visual tests to cover at least these snapshots:
- KCheckbox deselected
- KCheckbox indeterminate
- KCheckbox selected
- KCheckbox deselected disabled
- KCheckbox indeterminate disabled
- KCheckbox selected disabled
- KCheckbox without label (with showLabel=false)
- KCheckbox using the default slot to set the label.
- KCheckbox with description
Note
Since we currently dont have a test file for KCheckbox. Please move the current KCheckbox.vue file to be a folder KCheckbox/index.vue and create the test file in KCheckbox/__tests__/KCheckbox.spec.js
Acceptance criteria
- A new visual test suite is created for
KCheckbox. - The test includes all required snapshots.
- The test runs successfully and captures the expected snapshots.
@AlexVelezLl I would love to work on this issue. Please assign this to me.
Hey @you-think-you-know-me! For sure! I will assign this to you, please let us know any question :).
Hi @AlexVelezLl , in case @you-think-you-know-me is busy in some other engagements, I think i can give this issue a try!
Hey @Abhishek-Punhani, I have already done most of the work and will soon raise PR. I was busy with my exams, but now I am free.
No pressure @you-think-you-know-me
Hey @Abhishek-Punhani, I have already done most of the work and will soon raise PR. I was busy with my exams, but now I am free.
Oh sorry I didn’t see any linked PR to this issue.
Automatically unassigning @you-think-you-know-me due to no comments here, or updates on the associated pull request for 1 month. @you-think-you-know-me, if you're still interested in this issue or already have work in progress, please message us here, and we'll assign you again. Thank you!
Hey @AlexVelezLl ! I'd like to work on this. could you please assign it to me?
Hi @SukhvirKooner, I originally posted that we will assign to you, but now I noticed @you-think-you-know-me mentioned that most of the work is done and there were exams - so I think on this issue I will wait for a bit more to see if we hear back. If not, we will re-assign to you.
Sure! No Problem 👍
I will assign you now @SukhvirKooner
Thank you! @MisRob assigning this issue to me. I'll start working on this.
Hi @MisRob
Quick update on this issue
- Error:
nuxt_plugin_visualloadtestcomponents_* 500 errorduring the visual test setup when i ranyarn test:visual
I think 500 error happened because Nuxt only bundles plugins in its plugins/ directory (or ones explicitly listed in nuxt.config.js), and we had visual.load-test-components.js tucked away under jest.conf/. Nuxt generated an internal alias for that file but never actually included it, so every request for the visual playground blew up.
- Fix: I moved
visual.load-test-components.jsinto ourplugins/folder (underdocs/plugins/). In that plugin I globally register:
import KCheckbox from '~~/lib/KCheckbox/index.vue'
Vue.component('KCheckbox', KCheckbox);
This ensures <KCheckbox> renders in the testing playground instead of causing “unknown custom element” errors.
- Verification: After that, I ran the visual tests I wrote to cover all snapshots mentioned in the issue description KCheckbox deselected, indeterminate, selected, deselected disabled, indeterminate disabled, selected disabled, without label, using the default slot for the label and with description and they all passed without errors.
Questions before PR
- Should I keep visual.load-test-components.js in the plugins/ folder for this PR, or would you prefer it housed somewhere else or loaded via a different mechanism? Let me know your thoughts!
- Is there anything else I should cover or be aware of before opening the PR?
Thanks for reporting @SukhvirKooner. @AlexVelezLl do you have any thoughts on what would be best to do, or have you observed this for any other components?
@SukhvirKooner I think you can feel free to open a pull request anyway, and then this can be fine-tuned there.
Hi @SukhvirKooner! Apologies I missed this message. Can you add a capture of the error in the console?
In that plugin I globally register: import KCheckbox from '~~/lib/KCheckbox/index.vue' Vue.component('KCheckbox', KCheckbox);
You dont need to register KCheckbox for this, We just need to register Vue components that we use to render more complex layouts that we cant achieve with our renderComponentForVisualTest method, please see https://github.com/learningequality/kolibri-design-system/blob/develop/dev_docs/07_visual_testing_guide.md?plain=1#L104.
KCheckbox is a KDS component, we already declare this in our KThemePlugin, so we dont need to declare it again. Perhaps this is the reason why this was failing?
Just opened a PR to include more instructions about this ^ https://github.com/learningequality/kolibri-design-system/pull/1016.
thank you @AlexVelezLl for the guidance!
I’ve removed the unnecessary manual registration of KCheckbox:
import KCheckbox from '~~/lib/KCheckbox/index.vue';
Vue.component('KCheckbox', KCheckbox);
since it’s already provided by KThemePlugin. I’ve opened a PR with this visual tests, please take a look and let me know if anything else needs adjustment. Thanks!
Closed by https://github.com/learningequality/kolibri-design-system/pull/1017