From 0bfb6245f190be8b200c193054fcc38bb57d6eb5 Mon Sep 17 00:00:00 2001 From: "nantawat.poothong" Date: Fri, 10 Mar 2023 15:08:04 +0700 Subject: [PATCH 1/5] fix(card): overlay menu check value when item trigger --- packages/elements/src/card/index.ts | 10 +++++----- packages/elements/src/overlay-menu/index.ts | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/elements/src/card/index.ts b/packages/elements/src/card/index.ts index dcf08bc7cc..508a6bd489 100644 --- a/packages/elements/src/card/index.ts +++ b/packages/elements/src/card/index.ts @@ -15,7 +15,7 @@ import { isSlotEmpty } from '@refinitiv-ui/utils/is-slot-empty.js'; import type { Button } from '../button'; import type { OverlayMenu, OverlayMenuData } from '../overlay-menu'; import type { CardConfig } from './helpers/types'; -import type { OpenedChangedEvent } from '../events'; +import type { OpenedChangedEvent, ItemTriggerEvent } from '../events'; import '../label/index.js'; import '../button/index.js'; import '../overlay-menu/index.js'; @@ -155,10 +155,11 @@ export class Card extends BasicElement { /** * Close menu + * @param event Custom event * @returns {void} */ - private closeMenu (): void { - if (this.menuElement?.opened) { + private closeMenu (event: ItemTriggerEvent): void { + if (this.menuElement?.opened && event.detail.value) { this.menuElement.opened = false; this.menuOpened = false; } @@ -225,8 +226,7 @@ export class Card extends BasicElement { */ protected firstUpdated (changedProperties: PropertyValues): void { super.firstUpdated(changedProperties); - - this.addEventListener('item-trigger', this.closeMenu); // Here to cover nested menus + this.addEventListener('item-trigger', (event) => this.closeMenu(event as ItemTriggerEvent)); // Here to cover nested menus } /** diff --git a/packages/elements/src/overlay-menu/index.ts b/packages/elements/src/overlay-menu/index.ts index 1108d6a924..80b45b4a11 100644 --- a/packages/elements/src/overlay-menu/index.ts +++ b/packages/elements/src/overlay-menu/index.ts @@ -1029,7 +1029,7 @@ export class OverlayMenu extends Overlay { .label=${label} .subLabel=${subLabel} .icon=${icon} - .value=${value} + .value=${ifDefined(value)} .for=${ifDefined(forMenu || undefined)}> `; } From e88e0748368804b23f9e7f2e8ac0f86fb03e4fde Mon Sep 17 00:00:00 2001 From: "nantawat.poothong" Date: Fri, 10 Mar 2023 15:41:11 +0700 Subject: [PATCH 2/5] test(card): not close menu when click non value item --- .../elements/src/card/__test__/card.test.js | 32 ++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/packages/elements/src/card/__test__/card.test.js b/packages/elements/src/card/__test__/card.test.js index 26b80e202e..cbdf2f678a 100644 --- a/packages/elements/src/card/__test__/card.test.js +++ b/packages/elements/src/card/__test__/card.test.js @@ -10,7 +10,7 @@ import '@formatjs/intl-pluralrules/locale-data/en'; import '@refinitiv-ui/elements/card'; import '@refinitiv-ui/elemental-theme/light/ef-card'; -const menuData = [{ label: 'Spain', value: 'Spain' }, { label: 'France',value: 'France', disabled: true }, { label: 'Italy', value: 'Italy' }]; +const menuData = [{ label: 'Spain', value: 'Spain' }, { label: 'France',value: 'France', disabled: true }, { label: 'Italy', value: 'Italy' }, { label: 'Other', items: [{ label: 'Thailand', value: 'Thailand' }] }]; describe('card/Card', () => { describe('DOM structure', () => { @@ -82,6 +82,36 @@ describe('card/Card', () => { await oneEvent(el, 'item-trigger'); expect(menu.opened).to.equal(false, 'Menu should close when item is selected'); }); + it('Should open menu and not close when clicked on non value item', async () => { + const el = await fixture('Card'); + el.config = { + menu: { + data: menuData + } + }; + await elementUpdated(el); + + const openMenu = el.openMenuElement; + const menu = el.menuElement; + + expect(openMenu, 'Open menu button element does not exist').to.exist; + expect(menu, 'Menu element does not exist').to.exist; + + openMenu.click(); + + await elementUpdated(el); + expect(menu.opened).to.equal(true, 'Menu should open on button click'); + + const item = menu.shadowRoot.querySelectorAll('ef-item')[3]; + + expect(item, 'Menu config is not passed correctly').to.exist; + + setTimeout(() => { + item.click(); + }); + await oneEvent(el, 'item-trigger'); + expect(menu.opened).to.equal(true, 'Menu should still open'); + }); }); describe('Accessibility', () => { From b1cb3a103d0d3b54c2382ff29ae163215ca2c2c6 Mon Sep 17 00:00:00 2001 From: Nantawat Poothong <102957966+Nantawat-Poothong@users.noreply.github.com> Date: Thu, 16 Mar 2023 15:29:42 +0700 Subject: [PATCH 3/5] chore(card): update comment wording Co-authored-by: wattachai <117723407+wattachai-lseg@users.noreply.github.com> --- packages/elements/src/card/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/elements/src/card/index.ts b/packages/elements/src/card/index.ts index 508a6bd489..15f74934b6 100644 --- a/packages/elements/src/card/index.ts +++ b/packages/elements/src/card/index.ts @@ -155,7 +155,7 @@ export class Card extends BasicElement { /** * Close menu - * @param event Custom event + * @param event ItemTriggerEvent * @returns {void} */ private closeMenu (event: ItemTriggerEvent): void { From af0f50dea7c71b27613f44df94d6477c71b5fd17 Mon Sep 17 00:00:00 2001 From: "nantawat.poothong" Date: Thu, 16 Mar 2023 15:30:53 +0700 Subject: [PATCH 4/5] fix(card): update follow review suggestion --- packages/elements/src/card/__test__/card.test.js | 2 +- packages/elements/src/overlay-menu/index.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/elements/src/card/__test__/card.test.js b/packages/elements/src/card/__test__/card.test.js index cbdf2f678a..3a5838dd97 100644 --- a/packages/elements/src/card/__test__/card.test.js +++ b/packages/elements/src/card/__test__/card.test.js @@ -82,7 +82,7 @@ describe('card/Card', () => { await oneEvent(el, 'item-trigger'); expect(menu.opened).to.equal(false, 'Menu should close when item is selected'); }); - it('Should open menu and not close when clicked on non value item', async () => { + it('Should open menu and not close when clicked on non value item', async function () { const el = await fixture('Card'); el.config = { menu: { diff --git a/packages/elements/src/overlay-menu/index.ts b/packages/elements/src/overlay-menu/index.ts index 80b45b4a11..47efed9afe 100644 --- a/packages/elements/src/overlay-menu/index.ts +++ b/packages/elements/src/overlay-menu/index.ts @@ -1029,7 +1029,7 @@ export class OverlayMenu extends Overlay { .label=${label} .subLabel=${subLabel} .icon=${icon} - .value=${ifDefined(value)} + value=${ifDefined(value || undefined)} .for=${ifDefined(forMenu || undefined)}> `; } From 71686e2358b8a0af6233c45b8ebeb261d689b5d3 Mon Sep 17 00:00:00 2001 From: "nantawat.poothong" Date: Thu, 16 Mar 2023 15:32:31 +0700 Subject: [PATCH 5/5] fix(overlay-menu): wrong code syntax --- packages/elements/src/overlay-menu/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/elements/src/overlay-menu/index.ts b/packages/elements/src/overlay-menu/index.ts index 47efed9afe..6bca49fbe3 100644 --- a/packages/elements/src/overlay-menu/index.ts +++ b/packages/elements/src/overlay-menu/index.ts @@ -1029,7 +1029,7 @@ export class OverlayMenu extends Overlay { .label=${label} .subLabel=${subLabel} .icon=${icon} - value=${ifDefined(value || undefined)} + .value=${ifDefined(value || undefined)} .for=${ifDefined(forMenu || undefined)}> `; }