Add visual tests for KDropdownMenu (header, icons, multiple, single) and rename KDropdownMenu.vue to index.vue #933

GautamBytes · 2025-02-07T14:09:22Z

Description

This PR adds visual tests for the KDropdownMenu component, covering the following scenarios:

Rendering with a single item
Rendering with multiple items
Rendering items with icons
Rendering with a header slot

Additionally, the KDropdownMenu component file has been renamed from KDropdownMenu.vue to KDropdownMenu/index.vue to better organize the component structure.

Issue addressed

Closes #926

Before/after screenshots

NA

Changelog

Description: Added visual tests for KDropdownMenu (covering single, multiple, icons, and header slot scenarios) and renamed KDropdownMenu.vue to KDropdownMenu/index.vue.
Products impact: None
Addresses: [Visual testing]: Add visual tests for KDropdownMenu #926
Components: KDropdownMenu
Breaking: No
Impacts a11y: No
Guidance: Visual tests are implemented in separate test files following the approach used for KButton visual tests. This ensures that different configurations of KDropdownMenu render correctly and consistently.

Steps to test

Pull down the branch and build the project.
Run the visual tests using -- yarn test:visual
Verify that the visual tests for KDropdownMenu pass for:

Single item
Multiple items
Items with icons
With header slot

Check the Percy build to confirm that snapshots are captured correctly.

(optional) Implementation notes

At a high level, how did you implement this?

Added four separate visual test files for KDropdownMenu in lib/KDropdownMenu/tests/ to isolate each scenario.
The tests render KDropdownMenu via a KButton’s menu slot, simulating a user click to open the dropdown before taking snapshots.
The visual test utility was updated to accept an optional selector (knownSelector) to wait for an element (e.g., [data-test="dropdown-menu"]) indicating that the dropdown is rendered.
Renamed KDropdownMenu.vue to KDropdownMenu/index.vue for improved component organization.

Does this introduce any tech-debt items?

No, these changes follow the existing testing infrastructure and component guidelines.

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

Verify that the visual tests run as expected and that Percy snapshots accurately reflect the UI changes.
Confirm that renaming the component file to index.vue does not break any existing imports.
Ensure that the new tests follow the pattern used in the KButton visual tests.
[✅] Is the code clean and well-commented?
[✅] Are there tests for this change?

Comments

Please let me know if there are any questions or further adjustments needed.

…and rename KDropdownMenu.vue to index.vue

GautamBytes · 2025-02-07T14:19:48Z

Hello @AlexVelezLl , I have added the visual test for kdropdownmenu , it passed all 4 cases capturing the required snapshots. You can have a look and suggest me if anything further changes are required!

AlexVelezLl

Hi @GautamBytes! Thanks a lot for this! I have left an initial review :). If there are any questions, please let us know 👐.

AlexVelezLl · 2025-02-07T15:41:48Z

lib/KDropdownMenu/__tests__/KDropdownMenu.header.spec.js

@@ -0,0 +1,30 @@
+import { renderComponent, takeSnapshot, click } from '../../../jest.conf/visual.testUtils';


Lets keep the test suite just as the example I linked in the issue. So lets just have one KDropdownMenu.specs.js file, where we have this structure:

describe.visual('KDropdownMenu', () => { it(...visual test for test 1) it(...visual test for test 2) it(...visual test for test 3) it(...visual test for test 4) })

AlexVelezLl · 2025-02-07T15:45:27Z

jest.conf/visual.testUtils.js

+  // For KDropdownMenu, use a known selector from Keen's UI Menu.
+  // Adjust the selector as needed (e.g., it might be '.ui-menu', '.keen-ui-menu', or a data attribute).
+  const knownSelector =
+    component === 'KDropdownMenu'


As linked previously, we should be able to able to render the dropdown menu with the current implementation of our visual.testUtils, please lets try not to modify this file too much as right now is pretty stable, and if we do need to modify it, we should try to avoid hardcoding specific components in it, because this is a general file with general methods.

This is the rendered example of the linked test:

AlexVelezLl · 2025-02-07T15:47:41Z

jest.conf/visual.testUtils.js

- * Example:
- * {
- *   default: {
- *     element: "div", // or any Vue component like KIcon


Lets revert this change, please :). JsDocs are super super important for methods like this.

AlexVelezLl · 2025-02-07T15:48:45Z

jest.conf/visual.testUtils.js

  const beforeRenderState = await page.evaluate(() => {
    const testing_playground = document.querySelector('#testing-playground');
    return testing_playground ? testing_playground.innerHTML : '';
  });

+  // Trigger rendering via postMessage.


Lets revert these comments too.

AlexVelezLl · 2025-02-07T15:49:08Z

jest.conf/visual.testUtils.js

  );

-  // Check if the component has been rendered by comparing the initial state with the current state.


AlexVelezLl · 2025-02-07T15:49:32Z

jest.conf/visual.testUtils.js

 *
- * For a full list of available options, refer to the Percy documentation:


Idem. This is really really super important for other people to have a reference of the availabe option, its not here just because. :) Lets revert these changes, please 👐

AlexVelezLl · 2025-02-07T15:50:47Z

lib/KDropdownMenu/__tests__/KDropdownMenu.header.spec.js

+          },
+        },
+      },
+      dropdownSelector


We dont have a fourth argument for this method 😅

…ions

GautamBytes · 2025-02-09T17:29:29Z

Hey @AlexVelezLl , did the suggested changes , can you please check now and suggest if any improvements are needed. Thanks!
Attaching snapshots for reference)

GautamBytes · 2025-02-12T16:00:30Z

Hey @AlexVelezLl , I noticed there is a conflict in file -- lib/kdropdownmenu.vue , but i am unable to resolve the conflict and can't understand the cause , can you suggest me what to do?

RONAK-AI647 · 2025-02-12T16:05:44Z

Hey @AlexVelezLl , I noticed there is a conflict in file -- lib/kdropdownmenu.vue , but i am unable to resolve the conflict and can't understand the cause , can you suggest me what to do?

May be your branch is not up to date ..update it . Rest @AlexVelezLl could point out the accurate reason.

GautamBytes · 2025-02-12T16:08:29Z

It is up-to-date , already checked that

AlexVelezLl · 2025-02-13T18:10:02Z

Hey @GautamBytes! The conflict is because of an modify/rename file conflict, you can keep your current changes, and make sure that the changes introduced in https://github.com/learningequality/kolibri-design-system/pull/856/files are still available in the renamed file :).

On a side note. I opened this PR that will properly fix the remount of the components, so you wont need to override the styles with the percyCSS field. Once that PR is merged I will suggest some changes here 👐. Thanks in advance :).

GautamBytes · 2025-02-13T18:26:56Z

Sure @AlexVelezLl , will wait till your pr is merged and meanwhile i will check if the changes are kept in renamed file!

GautamBytes · 2025-02-22T09:57:42Z

Hey @AlexVelezLl , I have solved the conflict and also added the changes from your referenced pr to the renamed file . You can have a look and also suggest me what changes to do once your PR is merged.

Add visual tests for KDropdownMenu (header, icons, multiple, single) …

9629e7c

…and rename KDropdownMenu.vue to index.vue

GautamBytes requested a deployment to percy_tests February 7, 2025 14:09 — with GitHub Actions Waiting

AlexVelezLl requested changes Feb 7, 2025

View reviewed changes

Refactor KDropdownMenu tests into a single file and update configurat…

dd7a077

…ions

GautamBytes requested a deployment to percy_tests February 9, 2025 17:26 — with GitHub Actions Waiting

AlexVelezLl self-assigned this Feb 11, 2025

fix: adjust import paths for KDropdownMenu restructure

e5c4e1e

GautamBytes requested a deployment to percy_tests February 12, 2025 15:47 — with GitHub Actions Waiting

fix: adjust import paths for KDropdownMenu restructure

1fa995a

GautamBytes requested a deployment to percy_tests February 12, 2025 15:50 — with GitHub Actions Waiting

ensure changes are kept in renamed vue file

d633992

GautamBytes requested a deployment to percy_tests February 22, 2025 08:38 — with GitHub Actions Waiting

Resolve conflict in KDropdownMenu rename

a81d528

GautamBytes requested a deployment to percy_tests February 22, 2025 09:30 — with GitHub Actions Waiting

Rename kdropdownmenu.vue to index.vue and merge develop changes

157e50e

GautamBytes requested a deployment to percy_tests February 22, 2025 09:54 — with GitHub Actions Waiting

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add visual tests for KDropdownMenu (header, icons, multiple, single) and rename KDropdownMenu.vue to index.vue #933

Add visual tests for KDropdownMenu (header, icons, multiple, single) and rename KDropdownMenu.vue to index.vue #933

GautamBytes commented Feb 7, 2025 •

edited by AlexVelezLl

Loading

GautamBytes commented Feb 7, 2025

AlexVelezLl left a comment

AlexVelezLl Feb 7, 2025

AlexVelezLl Feb 7, 2025

AlexVelezLl Feb 7, 2025

AlexVelezLl Feb 7, 2025

AlexVelezLl Feb 7, 2025

AlexVelezLl Feb 7, 2025

AlexVelezLl Feb 7, 2025

GautamBytes commented Feb 9, 2025

GautamBytes commented Feb 12, 2025

RONAK-AI647 commented Feb 12, 2025

GautamBytes commented Feb 12, 2025

AlexVelezLl commented Feb 13, 2025

GautamBytes commented Feb 13, 2025

GautamBytes commented Feb 22, 2025

		@@ -0,0 +1,30 @@
		import { renderComponent, takeSnapshot, click } from '../../../jest.conf/visual.testUtils';

		);

		// Check if the component has been rendered by comparing the initial state with the current state.

		*
		* For a full list of available options, refer to the Percy documentation:

Add visual tests for KDropdownMenu (header, icons, multiple, single) and rename KDropdownMenu.vue to index.vue #933

Are you sure you want to change the base?

Add visual tests for KDropdownMenu (header, icons, multiple, single) and rename KDropdownMenu.vue to index.vue #933

Conversation

GautamBytes commented Feb 7, 2025 • edited by AlexVelezLl Loading

Description

Issue addressed

Before/after screenshots

Changelog

Steps to test

(optional) Implementation notes

At a high level, how did you implement this?

Does this introduce any tech-debt items?

Testing checklist

Reviewer guidance

Comments

GautamBytes commented Feb 7, 2025

AlexVelezLl left a comment

Choose a reason for hiding this comment

AlexVelezLl Feb 7, 2025

Choose a reason for hiding this comment

AlexVelezLl Feb 7, 2025

Choose a reason for hiding this comment

AlexVelezLl Feb 7, 2025

Choose a reason for hiding this comment

AlexVelezLl Feb 7, 2025

Choose a reason for hiding this comment

AlexVelezLl Feb 7, 2025

Choose a reason for hiding this comment

AlexVelezLl Feb 7, 2025

Choose a reason for hiding this comment

AlexVelezLl Feb 7, 2025

Choose a reason for hiding this comment

GautamBytes commented Feb 9, 2025

GautamBytes commented Feb 12, 2025

RONAK-AI647 commented Feb 12, 2025

GautamBytes commented Feb 12, 2025

AlexVelezLl commented Feb 13, 2025

GautamBytes commented Feb 13, 2025

GautamBytes commented Feb 22, 2025

GautamBytes commented Feb 7, 2025 •

edited by AlexVelezLl

Loading