From ea69c0bfd0d6042924bf49731521a64d03e72027 Mon Sep 17 00:00:00 2001
From: Goutham Ponnada <sponnada@ebay.com>
Date: Wed, 17 Jan 2024 20:09:51 -0800
Subject: [PATCH 1/5] feat(menu-button): added support for icons

---
 docs/makeup-menu-button/index.html            | 55 +++++++++++++++++++
 docs/makeup-menu-button/index.js              | 10 +++-
 docs/makeup-menu/index.html                   | 40 ++++++++++++++
 docs/makeup-menu/index.js                     | 10 +++-
 packages/makeup-menu-button/dist/cjs/index.js | 18 ++++--
 packages/makeup-menu-button/dist/mjs/index.js | 18 ++++--
 packages/makeup-menu-button/src/index.js      | 14 ++++-
 packages/makeup-menu/dist/cjs/index.js        | 26 +++++++--
 packages/makeup-menu/dist/mjs/index.js        | 19 +++++--
 packages/makeup-menu/src/index.js             | 19 ++++++-
 10 files changed, 204 insertions(+), 25 deletions(-)

diff --git a/docs/makeup-menu-button/index.html b/docs/makeup-menu-button/index.html
index 0d7920d4..11102d43 100644
--- a/docs/makeup-menu-button/index.html
+++ b/docs/makeup-menu-button/index.html
@@ -189,6 +189,61 @@ <h2 id="menu-button-2C">Single-Select Menu Button with Stateful Label</h2>
 
         <hr />
 
+        <h2 id="menu-button-2D">Single-Select Menu Button with Icons</h2>
+
+        <span class="menu-button menu-button-with-icon">
+          <button class="btn" type="button">
+            <span class="btn__text">
+              <svg class="icon icon--amex-18-colored" focusable="false" height="18" width="30" aria-hidden="true">
+                <use xlink:href="../icons.svg#icon-amex-18-colored"></use>
+              </svg>
+              <span>AMEX</span>
+            </span>
+            <svg focusable="false" height="10" width="14" aria-hidden="true">
+              <use xlink:href="../icons.svg#icon-chevron-down-12"></use>
+            </svg>
+          </button>
+          <div class="menu-button__menu" hidden>
+            <div class="menu-button__items" role="menu">
+              <div class="menu-button__item" role="menuitemradio" data-makeup-group="sort" aria-checked="true">
+                <div class="menu-button__item-value">
+                  <svg class="icon icon--amex-18-colored" focusable="false" height="18" width="30" aria-hidden="true">
+                    <use xlink:href="../icons.svg#icon-amex-18-colored"></use>
+                  </svg>
+                  <span>AMEX</span>
+                </div>
+                <svg class="icon icon--tick-16" focusable="false" height="10" width="14" aria-hidden="true">
+                  <use xlink:href="../icons.svg#icon-tick-16"></use>
+                </svg>
+              </div>
+              <div class="menu-button__item" role="menuitemradio" data-makeup-group="sort" aria-checked="false">
+                <div class="menu-button__item-value">
+                  <svg class="icon icon--visa-18-colored" focusable="false" height="18" width="30" aria-hidden="true">
+                    <use xlink:href="../icons.svg#icon-visa-18-colored"></use>
+                  </svg>
+                  <span>VISA</span>
+                </div>
+                <svg class="icon icon--tick-16" focusable="false" height="10" width="14" aria-hidden="true">
+                  <use xlink:href="../icons.svg#icon-tick-16"></use>
+                </svg>
+              </div>
+              <div class="menu-button__item" role="menuitemradio" data-makeup-group="sort" aria-checked="false">
+                <div class="menu-button__item-value">
+                  <svg class="icon icon--mastercard-18-colored" focusable="false" height="18" width="30" aria-hidden="true">
+                    <use xlink:href="../icons.svg#icon-mastercard-18-colored"></use>
+                  </svg>
+                  <span>Master Card</span>
+                </div>
+                <svg class="icon icon--tick-16" focusable="false" height="10" width="14" aria-hidden="true">
+                  <use xlink:href="../icons.svg#icon-tick-16"></use>
+                </svg>
+              </div>
+            </div>
+          </div>
+        </span>
+
+        <hr />
+
         <h2 id="menu-button-3">Multi-Select Menu Button</h2>
         <span class="menu-button">
           <button class="btn" aria-expanded="false" aria-haspopup="true" type="button">
diff --git a/docs/makeup-menu-button/index.js b/docs/makeup-menu-button/index.js
index ee8eebeb..c5f24d85 100644
--- a/docs/makeup-menu-button/index.js
+++ b/docs/makeup-menu-button/index.js
@@ -11,11 +11,19 @@ import "@ebay/skin/menu-button";
 import MenuButton from "../../packages/makeup-menu-button";
 
 window.onload = function () {
-  document.querySelectorAll(".menu-button").forEach(function (el, i) {
+  document.querySelectorAll(".menu-button:not(.menu-button-with-icon)").forEach(function (el, i) {
     const widget = new MenuButton(el);
 
     widget.menu.el.addEventListener("makeup-menu-select", (e) => console.log(e.type, e.detail));
     widget.menu.el.addEventListener("makeup-menu-change", (e) => console.log(e.type, e.detail));
     widget.menu.el.addEventListener("makeup-menu-button-mutation", (e) => console.log(e.type, e.detail));
   });
+
+  document.querySelectorAll(".menu-button-with-icon").forEach(function (el, i) {
+    const widget = new MenuButton(el, {valueTypeHTML: true});
+
+    widget.menu.el.addEventListener("makeup-menu-select", (e) => console.log(e.type, e.detail));
+    widget.menu.el.addEventListener("makeup-menu-change", (e) => console.log(e.type, e.detail));
+    widget.menu.el.addEventListener("makeup-menu-button-mutation", (e) => console.log(e.type, e.detail));
+  });
 };
diff --git a/docs/makeup-menu/index.html b/docs/makeup-menu/index.html
index d6b9c678..700700fa 100644
--- a/docs/makeup-menu/index.html
+++ b/docs/makeup-menu/index.html
@@ -117,6 +117,46 @@ <h2 id="menu-2b">Single-Select</h2>
           </div>
         </div>
 
+        <h2 id="menu-2c">Single-Select with Icons</h2>
+
+        <div class="menu menu-icon">
+          <div class="menu__items" role="menu">
+            <div class="menu__item" role="menuitemradio" data-makeup-group="credit-card" aria-checked="true">
+              <div class="menu__item-value">
+                <svg class="icon icon--amex-18-colored" focusable="false" height="18" width="30" aria-hidden="true">
+                  <use xlink:href="../icons.svg#icon-amex-18-colored"></use>
+                </svg>
+                <span>AMEX</span>
+              </div>
+              <svg class="icon icon--tick-16" focusable="false" height="10" width="14" aria-hidden="true">
+                <use xlink:href="../icons.svg#icon-tick-16"></use>
+              </svg>
+            </div>
+            <div class="menu__item" role="menuitemradio" data-makeup-group="credit-card" aria-checked="false">
+              <div class="menu__item-value">
+                <svg class="icon icon--visa-18-colored" focusable="false" height="18" width="30" aria-hidden="true">
+                  <use xlink:href="../icons.svg#icon-visa-18-colored"></use>
+                </svg>
+                <span>VISA</span>
+              </div>
+              <svg class="icon icon--tick-16" focusable="false" height="10" width="14" aria-hidden="true">
+                <use xlink:href="../icons.svg#icon-tick-16"></use>
+              </svg>
+            </div>
+            <div class="menu__item" role="menuitemradio" data-makeup-group="credit-card" aria-checked="false">
+              <div class="menu__item-value">
+                <svg class="icon icon--mastercard-18-colored" focusable="false" height="18" width="30" aria-hidden="true">
+                  <use xlink:href="../icons.svg#icon-mastercard-18-colored"></use>
+                </svg>
+                <span>Master Card</span>
+              </div>
+              <svg class="icon icon--tick-16" focusable="false" height="10" width="14" aria-hidden="true">
+                <use xlink:href="../icons.svg#icon-tick-16"></use>
+              </svg>
+            </div>
+          </div>
+        </div>
+
         <h2 id="menu-3">Multi-Select</h2>
         <div class="menu">
           <div class="menu__items" role="menu">
diff --git a/docs/makeup-menu/index.js b/docs/makeup-menu/index.js
index 4193274d..1fc8a989 100644
--- a/docs/makeup-menu/index.js
+++ b/docs/makeup-menu/index.js
@@ -12,11 +12,19 @@ import Menu from "../../packages/makeup-menu";
 const log = (e) => console.log(e.type, e.detail);
 
 window.onload = function () {
-  document.querySelectorAll(".menu").forEach(function (el, i) {
+  document.querySelectorAll(".menu:not(.menu-icon)").forEach(function (el, i) {
     const widget = new Menu(el);
 
     el.addEventListener("makeup-menu-select", log);
     el.addEventListener("makeup-menu-change", log);
     el.addEventListener("makeup-menu-mutation", log);
   });
+
+  document.querySelectorAll(".menu-icon").forEach(function (el, i) {
+    const widget = new Menu(el, { valueSelector: ".menu__item-value", valueTypeHTML: true });
+
+    el.addEventListener("makeup-menu-select", log);
+    el.addEventListener("makeup-menu-change", log);
+    el.addEventListener("makeup-menu-mutation", log);
+  });
 };
diff --git a/packages/makeup-menu-button/dist/cjs/index.js b/packages/makeup-menu-button/dist/cjs/index.js
index 921b8b3c..dba3a556 100644
--- a/packages/makeup-menu-button/dist/cjs/index.js
+++ b/packages/makeup-menu-button/dist/cjs/index.js
@@ -11,8 +11,12 @@ const defaultOptions = {
   customElementMode: false,
   expandedClass: "menu-button--expanded",
   menuSelector: ".menu-button__menu",
-  buttonTextSelector: ".btn__text"
+  buttonTextSelector: ".btn__text",
+  valueSelector: ".menu-button__item-value",
+  // Selector to get value from
+  valueTypeHTML: false // If true, will get innerHTML of valueSelector, otherwise will get innerText
 };
+
 class _default {
   constructor(widgetEl, selectedOptions) {
     var _this$_buttonEl$datas;
@@ -20,7 +24,9 @@ class _default {
     this.el = widgetEl;
     this._buttonEl = widgetEl.querySelector("button");
     this.menu = new _makeupMenu.default(widgetEl.querySelector(this._options.menuSelector), {
-      autoReset: "interactive"
+      autoReset: "interactive",
+      valueSelector: this._options.valueSelector,
+      valueTypeHTML: this._options.valueTypeHTML
     });
     this._buttonPrefix = (_this$_buttonEl$datas = this._buttonEl.dataset) === null || _this$_buttonEl$datas === void 0 ? void 0 : _this$_buttonEl$datas.makeupMenuButtonPrefix;
     this._buttonTextEl = this._buttonEl.querySelector(this._options.buttonTextSelector);
@@ -109,8 +115,12 @@ function _onMenuKeyDown(e) {
   }
 }
 function _onMenuItemSelect(e) {
-  if (this._buttonPrefix && e.detail.el.getAttribute("role") === "menuitemradio") {
-    this._buttonTextEl.innerText = "".concat(this._buttonPrefix, " ").concat(e.detail.el.innerText);
+  if (e.detail.el.getAttribute("role") === "menuitemradio") {
+    if (this._options.valueTypeHTML) {
+      this._buttonTextEl.innerHTML = e.detail.el.querySelector(this._options.valueSelector).innerHTML;
+    } else {
+      this._buttonTextEl.innerText = this._buttonPrefix ? "".concat(this._buttonPrefix, " ").concat(e.detail.el.innerText) : e.detail.el.innerText;
+    }
   }
   const widget = this;
   setTimeout(function () {
diff --git a/packages/makeup-menu-button/dist/mjs/index.js b/packages/makeup-menu-button/dist/mjs/index.js
index cb8819f9..c84b36ec 100644
--- a/packages/makeup-menu-button/dist/mjs/index.js
+++ b/packages/makeup-menu-button/dist/mjs/index.js
@@ -4,7 +4,11 @@ const defaultOptions = {
   customElementMode: false,
   expandedClass: "menu-button--expanded",
   menuSelector: ".menu-button__menu",
-  buttonTextSelector: `.btn__text`
+  buttonTextSelector: `.btn__text`,
+  valueSelector: ".menu-button__item-value",
+  // Selector to get value from
+  valueTypeHTML: false
+  // If true, will get innerHTML of valueSelector, otherwise will get innerText
 };
 class src_default {
   constructor(widgetEl, selectedOptions) {
@@ -12,7 +16,9 @@ class src_default {
     this.el = widgetEl;
     this._buttonEl = widgetEl.querySelector("button");
     this.menu = new Menu(widgetEl.querySelector(this._options.menuSelector), {
-      autoReset: "interactive"
+      autoReset: "interactive",
+      valueSelector: this._options.valueSelector,
+      valueTypeHTML: this._options.valueTypeHTML
     });
     this._buttonPrefix = this._buttonEl.dataset?.makeupMenuButtonPrefix;
     this._buttonTextEl = this._buttonEl.querySelector(this._options.buttonTextSelector);
@@ -100,8 +106,12 @@ function _onMenuKeyDown(e) {
   }
 }
 function _onMenuItemSelect(e) {
-  if (this._buttonPrefix && e.detail.el.getAttribute("role") === "menuitemradio") {
-    this._buttonTextEl.innerText = `${this._buttonPrefix} ${e.detail.el.innerText}`;
+  if (e.detail.el.getAttribute("role") === "menuitemradio") {
+    if (this._options.valueTypeHTML) {
+      this._buttonTextEl.innerHTML = e.detail.el.querySelector(this._options.valueSelector).innerHTML;
+    } else {
+      this._buttonTextEl.innerText = this._buttonPrefix ? `${this._buttonPrefix} ${e.detail.el.innerText}` : e.detail.el.innerText;
+    }
   }
   const widget = this;
   setTimeout(function() {
diff --git a/packages/makeup-menu-button/src/index.js b/packages/makeup-menu-button/src/index.js
index 11188673..bafee844 100644
--- a/packages/makeup-menu-button/src/index.js
+++ b/packages/makeup-menu-button/src/index.js
@@ -6,6 +6,8 @@ const defaultOptions = {
   expandedClass: "menu-button--expanded",
   menuSelector: ".menu-button__menu",
   buttonTextSelector: `.btn__text`,
+  valueSelector: ".menu-button__item-value", // Selector to get value from
+  valueTypeHTML: false, // If true, will get innerHTML of valueSelector, otherwise will get innerText
 };
 
 export default class {
@@ -15,6 +17,8 @@ export default class {
     this._buttonEl = widgetEl.querySelector("button");
     this.menu = new Menu(widgetEl.querySelector(this._options.menuSelector), {
       autoReset: "interactive",
+      valueSelector: this._options.valueSelector,
+      valueTypeHTML: this._options.valueTypeHTML,
     });
     this._buttonPrefix = this._buttonEl.dataset?.makeupMenuButtonPrefix;
     this._buttonTextEl = this._buttonEl.querySelector(this._options.buttonTextSelector);
@@ -117,8 +121,14 @@ function _onMenuKeyDown(e) {
 }
 
 function _onMenuItemSelect(e) {
-  if (this._buttonPrefix && e.detail.el.getAttribute("role") === "menuitemradio") {
-    this._buttonTextEl.innerText = `${this._buttonPrefix} ${e.detail.el.innerText}`;
+  if (e.detail.el.getAttribute("role") === "menuitemradio") {
+    if (this._options.valueTypeHTML) {
+      this._buttonTextEl.innerHTML = e.detail.el.querySelector(this._options.valueSelector).innerHTML;
+    } else {
+      this._buttonTextEl.innerText = this._buttonPrefix
+        ? `${this._buttonPrefix} ${e.detail.el.innerText}`
+        : e.detail.el.innerText;
+    }
   }
 
   const widget = this;
diff --git a/packages/makeup-menu/dist/cjs/index.js b/packages/makeup-menu/dist/cjs/index.js
index aa2b9ff0..b9c3901a 100644
--- a/packages/makeup-menu/dist/cjs/index.js
+++ b/packages/makeup-menu/dist/cjs/index.js
@@ -6,13 +6,17 @@ Object.defineProperty(exports, "__esModule", {
 exports.default = void 0;
 var RovingTabIndex = _interopRequireWildcard(require("makeup-roving-tabindex"));
 var PreventScrollKeys = _interopRequireWildcard(require("makeup-prevent-scroll-keys"));
-function _getRequireWildcardCache(e) { if ("function" != typeof WeakMap) return null; var r = new WeakMap(), t = new WeakMap(); return (_getRequireWildcardCache = function (e) { return e ? t : r; })(e); }
-function _interopRequireWildcard(e, r) { if (!r && e && e.__esModule) return e; if (null === e || "object" != typeof e && "function" != typeof e) return { default: e }; var t = _getRequireWildcardCache(r); if (t && t.has(e)) return t.get(e); var n = { __proto__: null }, a = Object.defineProperty && Object.getOwnPropertyDescriptor; for (var u in e) if ("default" !== u && Object.prototype.hasOwnProperty.call(e, u)) { var i = a ? Object.getOwnPropertyDescriptor(e, u) : null; i && (i.get || i.set) ? Object.defineProperty(n, u, i) : n[u] = e[u]; } return n.default = e, t && t.set(e, n), n; }
+function _getRequireWildcardCache(nodeInterop) { if (typeof WeakMap !== "function") return null; var cacheBabelInterop = new WeakMap(); var cacheNodeInterop = new WeakMap(); return (_getRequireWildcardCache = function (nodeInterop) { return nodeInterop ? cacheNodeInterop : cacheBabelInterop; })(nodeInterop); }
+function _interopRequireWildcard(obj, nodeInterop) { if (!nodeInterop && obj && obj.__esModule) { return obj; } if (obj === null || typeof obj !== "object" && typeof obj !== "function") { return { default: obj }; } var cache = _getRequireWildcardCache(nodeInterop); if (cache && cache.has(obj)) { return cache.get(obj); } var newObj = {}; var hasPropertyDescriptor = Object.defineProperty && Object.getOwnPropertyDescriptor; for (var key in obj) { if (key !== "default" && Object.prototype.hasOwnProperty.call(obj, key)) { var desc = hasPropertyDescriptor ? Object.getOwnPropertyDescriptor(obj, key) : null; if (desc && (desc.get || desc.set)) { Object.defineProperty(newObj, key, desc); } else { newObj[key] = obj[key]; } } } newObj.default = obj; if (cache) { cache.set(obj, newObj); } return newObj; }
 const defaultOptions = {
   customElementMode: false,
   autoInit: "interactive",
-  autoReset: "interactive"
+  autoReset: "interactive",
+  valueSelector: ".menu__item-value",
+  // Selector to get value from
+  valueTypeHTML: false // If true, will get innerHTML of valueSelector, otherwise will get innerText
 };
+
 class _default {
   constructor(widgetEl, selectedOptions) {
     this._options = Object.assign({}, defaultOptions, selectedOptions);
@@ -40,7 +44,7 @@ class _default {
         _selectMenuItemCheckbox(this.el, el);
         break;
       case "menuitemradio":
-        _selectMenuItemRadio(this.el, el);
+        _selectMenuItemRadio(this.el, el, this._options);
         break;
       default:
         _selectMenuItem(this.el, el);
@@ -148,20 +152,30 @@ function _selectMenuItemCheckbox(widgetEl, menuItemEl) {
     }));
   }
 }
-function _selectMenuItemRadio(widgetEl, menuItemEl) {
+function _selectMenuItemRadio(widgetEl, menuItemEl, options) {
   if (menuItemEl.getAttribute("aria-disabled") !== "true") {
     const groupName = menuItemEl.dataset.makeupGroup;
     const checkedEl = widgetEl.querySelector("[data-makeup-group=".concat(groupName, "][aria-checked=true]"));
     if (checkedEl) {
       checkedEl.setAttribute("aria-checked", "false");
     }
+    let value = menuItemEl.innerText;
+
+    // Check if value selector is present and use that to get value based on type instead
+    // If its not present, will default to innerText of the whole item
+    if (options.valueSelector) {
+      const valueSelector = menuItemEl.querySelector(options.valueSelector);
+      if (valueSelector) {
+        value = options.valueTypeHTML ? valueSelector.innerHTML : valueSelector.innerText;
+      }
+    }
     if (checkedEl !== menuItemEl) {
       menuItemEl.setAttribute("aria-checked", "true");
       widgetEl.dispatchEvent(new CustomEvent("makeup-menu-change", {
         detail: {
           el: menuItemEl,
           group: groupName,
-          value: menuItemEl.innerText
+          value
         }
       }));
     }
diff --git a/packages/makeup-menu/dist/mjs/index.js b/packages/makeup-menu/dist/mjs/index.js
index 64b46328..064b2330 100644
--- a/packages/makeup-menu/dist/mjs/index.js
+++ b/packages/makeup-menu/dist/mjs/index.js
@@ -3,7 +3,11 @@ import * as PreventScrollKeys from "makeup-prevent-scroll-keys";
 const defaultOptions = {
   customElementMode: false,
   autoInit: "interactive",
-  autoReset: "interactive"
+  autoReset: "interactive",
+  valueSelector: ".menu__item-value",
+  // Selector to get value from
+  valueTypeHTML: false
+  // If true, will get innerHTML of valueSelector, otherwise will get innerText
 };
 class src_default {
   constructor(widgetEl, selectedOptions) {
@@ -32,7 +36,7 @@ class src_default {
         _selectMenuItemCheckbox(this.el, el);
         break;
       case "menuitemradio":
-        _selectMenuItemRadio(this.el, el);
+        _selectMenuItemRadio(this.el, el, this._options);
         break;
       default:
         _selectMenuItem(this.el, el);
@@ -143,13 +147,20 @@ function _selectMenuItemCheckbox(widgetEl, menuItemEl) {
     );
   }
 }
-function _selectMenuItemRadio(widgetEl, menuItemEl) {
+function _selectMenuItemRadio(widgetEl, menuItemEl, options) {
   if (menuItemEl.getAttribute("aria-disabled") !== "true") {
     const groupName = menuItemEl.dataset.makeupGroup;
     const checkedEl = widgetEl.querySelector(`[data-makeup-group=${groupName}][aria-checked=true]`);
     if (checkedEl) {
       checkedEl.setAttribute("aria-checked", "false");
     }
+    let value = menuItemEl.innerText;
+    if (options.valueSelector) {
+      const valueSelector = menuItemEl.querySelector(options.valueSelector);
+      if (valueSelector) {
+        value = options.valueTypeHTML ? valueSelector.innerHTML : valueSelector.innerText;
+      }
+    }
     if (checkedEl !== menuItemEl) {
       menuItemEl.setAttribute("aria-checked", "true");
       widgetEl.dispatchEvent(
@@ -157,7 +168,7 @@ function _selectMenuItemRadio(widgetEl, menuItemEl) {
           detail: {
             el: menuItemEl,
             group: groupName,
-            value: menuItemEl.innerText
+            value
           }
         })
       );
diff --git a/packages/makeup-menu/src/index.js b/packages/makeup-menu/src/index.js
index 744c2c60..01423fa0 100644
--- a/packages/makeup-menu/src/index.js
+++ b/packages/makeup-menu/src/index.js
@@ -5,6 +5,8 @@ const defaultOptions = {
   customElementMode: false,
   autoInit: "interactive",
   autoReset: "interactive",
+  valueSelector: ".menu__item-value", // Selector to get value from
+  valueTypeHTML: false, // If true, will get innerHTML of valueSelector, otherwise will get innerText
 };
 
 export default class {
@@ -43,7 +45,7 @@ export default class {
         _selectMenuItemCheckbox(this.el, el);
         break;
       case "menuitemradio":
-        _selectMenuItemRadio(this.el, el);
+        _selectMenuItemRadio(this.el, el, this._options);
         break;
       default:
         _selectMenuItem(this.el, el);
@@ -181,7 +183,7 @@ function _selectMenuItemCheckbox(widgetEl, menuItemEl) {
   }
 }
 
-function _selectMenuItemRadio(widgetEl, menuItemEl) {
+function _selectMenuItemRadio(widgetEl, menuItemEl, options) {
   if (menuItemEl.getAttribute("aria-disabled") !== "true") {
     const groupName = menuItemEl.dataset.makeupGroup;
     const checkedEl = widgetEl.querySelector(`[data-makeup-group=${groupName}][aria-checked=true]`);
@@ -190,6 +192,17 @@ function _selectMenuItemRadio(widgetEl, menuItemEl) {
       checkedEl.setAttribute("aria-checked", "false");
     }
 
+    let value = menuItemEl.innerText;
+
+    // Check if value selector is present and use that to get value based on type instead
+    // If its not present, will default to innerText of the whole item
+    if (options.valueSelector) {
+      const valueSelector = menuItemEl.querySelector(options.valueSelector);
+      if (valueSelector) {
+        value = options.valueTypeHTML ? valueSelector.innerHTML : valueSelector.innerText;
+      }
+    }
+
     if (checkedEl !== menuItemEl) {
       menuItemEl.setAttribute("aria-checked", "true");
 
@@ -198,7 +211,7 @@ function _selectMenuItemRadio(widgetEl, menuItemEl) {
           detail: {
             el: menuItemEl,
             group: groupName,
-            value: menuItemEl.innerText,
+            value,
           },
         }),
       );

From 92c49c001b635cf10b261e4af0e044707cd77cb9 Mon Sep 17 00:00:00 2001
From: Goutham Ponnada <sponnada@ebay.com>
Date: Thu, 18 Jan 2024 15:19:36 -0800
Subject: [PATCH 2/5] fix: dependency on configurations

---
 packages/makeup-menu-button/src/index.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/makeup-menu-button/src/index.js b/packages/makeup-menu-button/src/index.js
index bafee844..3c7b8f7d 100644
--- a/packages/makeup-menu-button/src/index.js
+++ b/packages/makeup-menu-button/src/index.js
@@ -122,7 +122,7 @@ function _onMenuKeyDown(e) {
 
 function _onMenuItemSelect(e) {
   if (e.detail.el.getAttribute("role") === "menuitemradio") {
-    if (this._options.valueTypeHTML) {
+    if (this._options.valueTypeHTML && this._options.valueSelector) {
       this._buttonTextEl.innerHTML = e.detail.el.querySelector(this._options.valueSelector).innerHTML;
     } else {
       this._buttonTextEl.innerText = this._buttonPrefix

From 57fbe181f29d36cb09a1a1592266d99786e8d2b0 Mon Sep 17 00:00:00 2001
From: Goutham Ponnada <sponnada@ebay.com>
Date: Tue, 23 Jan 2024 12:33:13 -0800
Subject: [PATCH 3/5] refactor: code review suggestions

---
 docs/makeup-menu-button/index.html            |  6 ++---
 docs/makeup-menu-button/index.js              | 12 ++-------
 docs/makeup-menu/index.js                     | 10 +------
 packages/makeup-menu-button/dist/cjs/index.js | 18 +++++++------
 packages/makeup-menu-button/dist/mjs/index.js | 17 +++++++-----
 packages/makeup-menu-button/src/index.js      | 22 +++++++++-------
 packages/makeup-menu/dist/cjs/index.js        | 26 +++++--------------
 packages/makeup-menu/dist/mjs/index.js        | 19 +++-----------
 packages/makeup-menu/src/index.js             | 19 +++-----------
 9 files changed, 51 insertions(+), 98 deletions(-)

diff --git a/docs/makeup-menu-button/index.html b/docs/makeup-menu-button/index.html
index 11102d43..ba8ee3c5 100644
--- a/docs/makeup-menu-button/index.html
+++ b/docs/makeup-menu-button/index.html
@@ -205,7 +205,7 @@ <h2 id="menu-button-2D">Single-Select Menu Button with Icons</h2>
           </button>
           <div class="menu-button__menu" hidden>
             <div class="menu-button__items" role="menu">
-              <div class="menu-button__item" role="menuitemradio" data-makeup-group="sort" aria-checked="true">
+              <div class="menu-button__item" role="menuitemradio" data-makeup-group="credit-card" aria-checked="true">
                 <div class="menu-button__item-value">
                   <svg class="icon icon--amex-18-colored" focusable="false" height="18" width="30" aria-hidden="true">
                     <use xlink:href="../icons.svg#icon-amex-18-colored"></use>
@@ -216,7 +216,7 @@ <h2 id="menu-button-2D">Single-Select Menu Button with Icons</h2>
                   <use xlink:href="../icons.svg#icon-tick-16"></use>
                 </svg>
               </div>
-              <div class="menu-button__item" role="menuitemradio" data-makeup-group="sort" aria-checked="false">
+              <div class="menu-button__item" role="menuitemradio" data-makeup-group="credit-card" aria-checked="false">
                 <div class="menu-button__item-value">
                   <svg class="icon icon--visa-18-colored" focusable="false" height="18" width="30" aria-hidden="true">
                     <use xlink:href="../icons.svg#icon-visa-18-colored"></use>
@@ -227,7 +227,7 @@ <h2 id="menu-button-2D">Single-Select Menu Button with Icons</h2>
                   <use xlink:href="../icons.svg#icon-tick-16"></use>
                 </svg>
               </div>
-              <div class="menu-button__item" role="menuitemradio" data-makeup-group="sort" aria-checked="false">
+              <div class="menu-button__item" role="menuitemradio" data-makeup-group="credit-card" aria-checked="false">
                 <div class="menu-button__item-value">
                   <svg class="icon icon--mastercard-18-colored" focusable="false" height="18" width="30" aria-hidden="true">
                     <use xlink:href="../icons.svg#icon-mastercard-18-colored"></use>
diff --git a/docs/makeup-menu-button/index.js b/docs/makeup-menu-button/index.js
index c5f24d85..45e7ff4b 100644
--- a/docs/makeup-menu-button/index.js
+++ b/docs/makeup-menu-button/index.js
@@ -11,16 +11,8 @@ import "@ebay/skin/menu-button";
 import MenuButton from "../../packages/makeup-menu-button";
 
 window.onload = function () {
-  document.querySelectorAll(".menu-button:not(.menu-button-with-icon)").forEach(function (el, i) {
-    const widget = new MenuButton(el);
-
-    widget.menu.el.addEventListener("makeup-menu-select", (e) => console.log(e.type, e.detail));
-    widget.menu.el.addEventListener("makeup-menu-change", (e) => console.log(e.type, e.detail));
-    widget.menu.el.addEventListener("makeup-menu-button-mutation", (e) => console.log(e.type, e.detail));
-  });
-
-  document.querySelectorAll(".menu-button-with-icon").forEach(function (el, i) {
-    const widget = new MenuButton(el, {valueTypeHTML: true});
+  document.querySelectorAll(".menu-button").forEach(function (el, i) {
+    const widget = new MenuButton(el, el.classList.contains("menu-button-with-icon") && {valueTypeHTML: true});
 
     widget.menu.el.addEventListener("makeup-menu-select", (e) => console.log(e.type, e.detail));
     widget.menu.el.addEventListener("makeup-menu-change", (e) => console.log(e.type, e.detail));
diff --git a/docs/makeup-menu/index.js b/docs/makeup-menu/index.js
index 1fc8a989..4193274d 100644
--- a/docs/makeup-menu/index.js
+++ b/docs/makeup-menu/index.js
@@ -12,19 +12,11 @@ import Menu from "../../packages/makeup-menu";
 const log = (e) => console.log(e.type, e.detail);
 
 window.onload = function () {
-  document.querySelectorAll(".menu:not(.menu-icon)").forEach(function (el, i) {
+  document.querySelectorAll(".menu").forEach(function (el, i) {
     const widget = new Menu(el);
 
     el.addEventListener("makeup-menu-select", log);
     el.addEventListener("makeup-menu-change", log);
     el.addEventListener("makeup-menu-mutation", log);
   });
-
-  document.querySelectorAll(".menu-icon").forEach(function (el, i) {
-    const widget = new Menu(el, { valueSelector: ".menu__item-value", valueTypeHTML: true });
-
-    el.addEventListener("makeup-menu-select", log);
-    el.addEventListener("makeup-menu-change", log);
-    el.addEventListener("makeup-menu-mutation", log);
-  });
 };
diff --git a/packages/makeup-menu-button/dist/cjs/index.js b/packages/makeup-menu-button/dist/cjs/index.js
index dba3a556..081d958b 100644
--- a/packages/makeup-menu-button/dist/cjs/index.js
+++ b/packages/makeup-menu-button/dist/cjs/index.js
@@ -16,7 +16,6 @@ const defaultOptions = {
   // Selector to get value from
   valueTypeHTML: false // If true, will get innerHTML of valueSelector, otherwise will get innerText
 };
-
 class _default {
   constructor(widgetEl, selectedOptions) {
     var _this$_buttonEl$datas;
@@ -115,16 +114,19 @@ function _onMenuKeyDown(e) {
   }
 }
 function _onMenuItemSelect(e) {
-  if (e.detail.el.getAttribute("role") === "menuitemradio") {
-    if (this._options.valueTypeHTML) {
-      this._buttonTextEl.innerHTML = e.detail.el.querySelector(this._options.valueSelector).innerHTML;
-    } else {
-      this._buttonTextEl.innerText = this._buttonPrefix ? "".concat(this._buttonPrefix, " ").concat(e.detail.el.innerText) : e.detail.el.innerText;
-    }
-  }
   const widget = this;
   setTimeout(function () {
     widget._expander.expanded = false;
     widget._buttonEl.focus();
   }, 150);
+  if (e.detail.el.getAttribute("role") !== "menuitemradio") {
+    return;
+  }
+  const valueSelector = e.detail.el.querySelector(this._options.valueSelector);
+  if (valueSelector) {
+    const valueType = this._options.valueTypeHTML ? "innerHTML" : "innerText";
+    this._buttonTextEl[valueType] = valueSelector[valueType];
+  } else if (this._buttonPrefix) {
+    this._buttonTextEl.innerText = "".concat(this._buttonPrefix, " ").concat(e.detail.el.innerText);
+  }
 }
diff --git a/packages/makeup-menu-button/dist/mjs/index.js b/packages/makeup-menu-button/dist/mjs/index.js
index c84b36ec..bda6addd 100644
--- a/packages/makeup-menu-button/dist/mjs/index.js
+++ b/packages/makeup-menu-button/dist/mjs/index.js
@@ -106,18 +106,21 @@ function _onMenuKeyDown(e) {
   }
 }
 function _onMenuItemSelect(e) {
-  if (e.detail.el.getAttribute("role") === "menuitemradio") {
-    if (this._options.valueTypeHTML) {
-      this._buttonTextEl.innerHTML = e.detail.el.querySelector(this._options.valueSelector).innerHTML;
-    } else {
-      this._buttonTextEl.innerText = this._buttonPrefix ? `${this._buttonPrefix} ${e.detail.el.innerText}` : e.detail.el.innerText;
-    }
-  }
   const widget = this;
   setTimeout(function() {
     widget._expander.expanded = false;
     widget._buttonEl.focus();
   }, 150);
+  if (e.detail.el.getAttribute("role") !== "menuitemradio") {
+    return;
+  }
+  const valueSelector = e.detail.el.querySelector(this._options.valueSelector);
+  if (valueSelector) {
+    const valueType = this._options.valueTypeHTML ? "innerHTML" : "innerText";
+    this._buttonTextEl[valueType] = valueSelector[valueType];
+  } else if (this._buttonPrefix) {
+    this._buttonTextEl.innerText = `${this._buttonPrefix} ${e.detail.el.innerText}`;
+  }
 }
 export {
   src_default as default
diff --git a/packages/makeup-menu-button/src/index.js b/packages/makeup-menu-button/src/index.js
index 3c7b8f7d..35d76774 100644
--- a/packages/makeup-menu-button/src/index.js
+++ b/packages/makeup-menu-button/src/index.js
@@ -121,20 +121,22 @@ function _onMenuKeyDown(e) {
 }
 
 function _onMenuItemSelect(e) {
-  if (e.detail.el.getAttribute("role") === "menuitemradio") {
-    if (this._options.valueTypeHTML && this._options.valueSelector) {
-      this._buttonTextEl.innerHTML = e.detail.el.querySelector(this._options.valueSelector).innerHTML;
-    } else {
-      this._buttonTextEl.innerText = this._buttonPrefix
-        ? `${this._buttonPrefix} ${e.detail.el.innerText}`
-        : e.detail.el.innerText;
-    }
-  }
-
   const widget = this;
 
   setTimeout(function () {
     widget._expander.expanded = false;
     widget._buttonEl.focus();
   }, 150);
+
+  if (e.detail.el.getAttribute("role") !== "menuitemradio") {
+    return;
+  }
+
+  const valueSelector = e.detail.el.querySelector(this._options.valueSelector);
+  if (valueSelector) {
+    const valueType = this._options.valueTypeHTML ? "innerHTML" : "innerText";
+    this._buttonTextEl[valueType] = valueSelector[valueType];
+  } else if (this._buttonPrefix) {
+    this._buttonTextEl.innerText = `${this._buttonPrefix} ${e.detail.el.innerText}`;
+  }
 }
diff --git a/packages/makeup-menu/dist/cjs/index.js b/packages/makeup-menu/dist/cjs/index.js
index b9c3901a..aa2b9ff0 100644
--- a/packages/makeup-menu/dist/cjs/index.js
+++ b/packages/makeup-menu/dist/cjs/index.js
@@ -6,17 +6,13 @@ Object.defineProperty(exports, "__esModule", {
 exports.default = void 0;
 var RovingTabIndex = _interopRequireWildcard(require("makeup-roving-tabindex"));
 var PreventScrollKeys = _interopRequireWildcard(require("makeup-prevent-scroll-keys"));
-function _getRequireWildcardCache(nodeInterop) { if (typeof WeakMap !== "function") return null; var cacheBabelInterop = new WeakMap(); var cacheNodeInterop = new WeakMap(); return (_getRequireWildcardCache = function (nodeInterop) { return nodeInterop ? cacheNodeInterop : cacheBabelInterop; })(nodeInterop); }
-function _interopRequireWildcard(obj, nodeInterop) { if (!nodeInterop && obj && obj.__esModule) { return obj; } if (obj === null || typeof obj !== "object" && typeof obj !== "function") { return { default: obj }; } var cache = _getRequireWildcardCache(nodeInterop); if (cache && cache.has(obj)) { return cache.get(obj); } var newObj = {}; var hasPropertyDescriptor = Object.defineProperty && Object.getOwnPropertyDescriptor; for (var key in obj) { if (key !== "default" && Object.prototype.hasOwnProperty.call(obj, key)) { var desc = hasPropertyDescriptor ? Object.getOwnPropertyDescriptor(obj, key) : null; if (desc && (desc.get || desc.set)) { Object.defineProperty(newObj, key, desc); } else { newObj[key] = obj[key]; } } } newObj.default = obj; if (cache) { cache.set(obj, newObj); } return newObj; }
+function _getRequireWildcardCache(e) { if ("function" != typeof WeakMap) return null; var r = new WeakMap(), t = new WeakMap(); return (_getRequireWildcardCache = function (e) { return e ? t : r; })(e); }
+function _interopRequireWildcard(e, r) { if (!r && e && e.__esModule) return e; if (null === e || "object" != typeof e && "function" != typeof e) return { default: e }; var t = _getRequireWildcardCache(r); if (t && t.has(e)) return t.get(e); var n = { __proto__: null }, a = Object.defineProperty && Object.getOwnPropertyDescriptor; for (var u in e) if ("default" !== u && Object.prototype.hasOwnProperty.call(e, u)) { var i = a ? Object.getOwnPropertyDescriptor(e, u) : null; i && (i.get || i.set) ? Object.defineProperty(n, u, i) : n[u] = e[u]; } return n.default = e, t && t.set(e, n), n; }
 const defaultOptions = {
   customElementMode: false,
   autoInit: "interactive",
-  autoReset: "interactive",
-  valueSelector: ".menu__item-value",
-  // Selector to get value from
-  valueTypeHTML: false // If true, will get innerHTML of valueSelector, otherwise will get innerText
+  autoReset: "interactive"
 };
-
 class _default {
   constructor(widgetEl, selectedOptions) {
     this._options = Object.assign({}, defaultOptions, selectedOptions);
@@ -44,7 +40,7 @@ class _default {
         _selectMenuItemCheckbox(this.el, el);
         break;
       case "menuitemradio":
-        _selectMenuItemRadio(this.el, el, this._options);
+        _selectMenuItemRadio(this.el, el);
         break;
       default:
         _selectMenuItem(this.el, el);
@@ -152,30 +148,20 @@ function _selectMenuItemCheckbox(widgetEl, menuItemEl) {
     }));
   }
 }
-function _selectMenuItemRadio(widgetEl, menuItemEl, options) {
+function _selectMenuItemRadio(widgetEl, menuItemEl) {
   if (menuItemEl.getAttribute("aria-disabled") !== "true") {
     const groupName = menuItemEl.dataset.makeupGroup;
     const checkedEl = widgetEl.querySelector("[data-makeup-group=".concat(groupName, "][aria-checked=true]"));
     if (checkedEl) {
       checkedEl.setAttribute("aria-checked", "false");
     }
-    let value = menuItemEl.innerText;
-
-    // Check if value selector is present and use that to get value based on type instead
-    // If its not present, will default to innerText of the whole item
-    if (options.valueSelector) {
-      const valueSelector = menuItemEl.querySelector(options.valueSelector);
-      if (valueSelector) {
-        value = options.valueTypeHTML ? valueSelector.innerHTML : valueSelector.innerText;
-      }
-    }
     if (checkedEl !== menuItemEl) {
       menuItemEl.setAttribute("aria-checked", "true");
       widgetEl.dispatchEvent(new CustomEvent("makeup-menu-change", {
         detail: {
           el: menuItemEl,
           group: groupName,
-          value
+          value: menuItemEl.innerText
         }
       }));
     }
diff --git a/packages/makeup-menu/dist/mjs/index.js b/packages/makeup-menu/dist/mjs/index.js
index 064b2330..64b46328 100644
--- a/packages/makeup-menu/dist/mjs/index.js
+++ b/packages/makeup-menu/dist/mjs/index.js
@@ -3,11 +3,7 @@ import * as PreventScrollKeys from "makeup-prevent-scroll-keys";
 const defaultOptions = {
   customElementMode: false,
   autoInit: "interactive",
-  autoReset: "interactive",
-  valueSelector: ".menu__item-value",
-  // Selector to get value from
-  valueTypeHTML: false
-  // If true, will get innerHTML of valueSelector, otherwise will get innerText
+  autoReset: "interactive"
 };
 class src_default {
   constructor(widgetEl, selectedOptions) {
@@ -36,7 +32,7 @@ class src_default {
         _selectMenuItemCheckbox(this.el, el);
         break;
       case "menuitemradio":
-        _selectMenuItemRadio(this.el, el, this._options);
+        _selectMenuItemRadio(this.el, el);
         break;
       default:
         _selectMenuItem(this.el, el);
@@ -147,20 +143,13 @@ function _selectMenuItemCheckbox(widgetEl, menuItemEl) {
     );
   }
 }
-function _selectMenuItemRadio(widgetEl, menuItemEl, options) {
+function _selectMenuItemRadio(widgetEl, menuItemEl) {
   if (menuItemEl.getAttribute("aria-disabled") !== "true") {
     const groupName = menuItemEl.dataset.makeupGroup;
     const checkedEl = widgetEl.querySelector(`[data-makeup-group=${groupName}][aria-checked=true]`);
     if (checkedEl) {
       checkedEl.setAttribute("aria-checked", "false");
     }
-    let value = menuItemEl.innerText;
-    if (options.valueSelector) {
-      const valueSelector = menuItemEl.querySelector(options.valueSelector);
-      if (valueSelector) {
-        value = options.valueTypeHTML ? valueSelector.innerHTML : valueSelector.innerText;
-      }
-    }
     if (checkedEl !== menuItemEl) {
       menuItemEl.setAttribute("aria-checked", "true");
       widgetEl.dispatchEvent(
@@ -168,7 +157,7 @@ function _selectMenuItemRadio(widgetEl, menuItemEl, options) {
           detail: {
             el: menuItemEl,
             group: groupName,
-            value
+            value: menuItemEl.innerText
           }
         })
       );
diff --git a/packages/makeup-menu/src/index.js b/packages/makeup-menu/src/index.js
index 01423fa0..744c2c60 100644
--- a/packages/makeup-menu/src/index.js
+++ b/packages/makeup-menu/src/index.js
@@ -5,8 +5,6 @@ const defaultOptions = {
   customElementMode: false,
   autoInit: "interactive",
   autoReset: "interactive",
-  valueSelector: ".menu__item-value", // Selector to get value from
-  valueTypeHTML: false, // If true, will get innerHTML of valueSelector, otherwise will get innerText
 };
 
 export default class {
@@ -45,7 +43,7 @@ export default class {
         _selectMenuItemCheckbox(this.el, el);
         break;
       case "menuitemradio":
-        _selectMenuItemRadio(this.el, el, this._options);
+        _selectMenuItemRadio(this.el, el);
         break;
       default:
         _selectMenuItem(this.el, el);
@@ -183,7 +181,7 @@ function _selectMenuItemCheckbox(widgetEl, menuItemEl) {
   }
 }
 
-function _selectMenuItemRadio(widgetEl, menuItemEl, options) {
+function _selectMenuItemRadio(widgetEl, menuItemEl) {
   if (menuItemEl.getAttribute("aria-disabled") !== "true") {
     const groupName = menuItemEl.dataset.makeupGroup;
     const checkedEl = widgetEl.querySelector(`[data-makeup-group=${groupName}][aria-checked=true]`);
@@ -192,17 +190,6 @@ function _selectMenuItemRadio(widgetEl, menuItemEl, options) {
       checkedEl.setAttribute("aria-checked", "false");
     }
 
-    let value = menuItemEl.innerText;
-
-    // Check if value selector is present and use that to get value based on type instead
-    // If its not present, will default to innerText of the whole item
-    if (options.valueSelector) {
-      const valueSelector = menuItemEl.querySelector(options.valueSelector);
-      if (valueSelector) {
-        value = options.valueTypeHTML ? valueSelector.innerHTML : valueSelector.innerText;
-      }
-    }
-
     if (checkedEl !== menuItemEl) {
       menuItemEl.setAttribute("aria-checked", "true");
 
@@ -211,7 +198,7 @@ function _selectMenuItemRadio(widgetEl, menuItemEl, options) {
           detail: {
             el: menuItemEl,
             group: groupName,
-            value,
+            value: menuItemEl.innerText,
           },
         }),
       );

From b7125b932a81e7ebfb8eee4334ee0959205de64a Mon Sep 17 00:00:00 2001
From: Goutham Ponnada <sponnada@ebay.com>
Date: Thu, 25 Jan 2024 20:59:15 -0800
Subject: [PATCH 4/5] refactor: code refactor contd..

---
 docs/makeup-menu-button/index.html            | 56 ++++++++++++++++++-
 docs/makeup-menu-button/index.js              | 11 +++-
 packages/makeup-menu-button/dist/cjs/index.js | 33 +++++++----
 packages/makeup-menu-button/dist/mjs/index.js | 34 ++++++-----
 packages/makeup-menu-button/src/index.js      | 31 ++++++----
 5 files changed, 126 insertions(+), 39 deletions(-)

diff --git a/docs/makeup-menu-button/index.html b/docs/makeup-menu-button/index.html
index ba8ee3c5..78f0ebec 100644
--- a/docs/makeup-menu-button/index.html
+++ b/docs/makeup-menu-button/index.html
@@ -192,6 +192,60 @@ <h2 id="menu-button-2C">Single-Select Menu Button with Stateful Label</h2>
         <h2 id="menu-button-2D">Single-Select Menu Button with Icons</h2>
 
         <span class="menu-button menu-button-with-icon">
+          <button class="btn" type="button">
+            <span class="btn__text">
+              <svg class="icon icon--amex-18-colored" focusable="false" height="18" width="30" aria-label="AMEX">
+                <use xlink:href="../icons.svg#icon-amex-18-colored"></use>
+              </svg>
+            </span>
+            <svg focusable="false" height="10" width="14" aria-hidden="true">
+              <use xlink:href="../icons.svg#icon-chevron-down-12"></use>
+            </svg>
+          </button>
+          <div class="menu-button__menu" hidden>
+            <div class="menu-button__items" role="menu">
+              <div class="menu-button__item" role="menuitemradio" data-makeup-group="credit-card" aria-checked="true">
+                <div class="menu-button__item-value">
+                  <svg class="icon icon--amex-18-colored" focusable="false" height="18" width="30" aria-hidden="true">
+                    <use xlink:href="../icons.svg#icon-amex-18-colored"></use>
+                  </svg>
+                  <span>AMEX</span>
+                </div>
+                <svg class="icon icon--tick-16" focusable="false" height="10" width="14" aria-hidden="true">
+                  <use xlink:href="../icons.svg#icon-tick-16"></use>
+                </svg>
+              </div>
+              <div class="menu-button__item" role="menuitemradio" data-makeup-group="credit-card" aria-checked="false">
+                <div class="menu-button__item-value">
+                  <svg class="icon icon--visa-18-colored" focusable="false" height="18" width="30" aria-hidden="true">
+                    <use xlink:href="../icons.svg#icon-visa-18-colored"></use>
+                  </svg>
+                  <span>VISA</span>
+                </div>
+                <svg class="icon icon--tick-16" focusable="false" height="10" width="14" aria-hidden="true">
+                  <use xlink:href="../icons.svg#icon-tick-16"></use>
+                </svg>
+              </div>
+              <div class="menu-button__item" role="menuitemradio" data-makeup-group="credit-card" aria-checked="false">
+                <div class="menu-button__item-value">
+                  <svg class="icon icon--mastercard-18-colored" focusable="false" height="18" width="30" aria-hidden="true">
+                    <use xlink:href="../icons.svg#icon-mastercard-18-colored"></use>
+                  </svg>
+                  <span>Master Card</span>
+                </div>
+                <svg class="icon icon--tick-16" focusable="false" height="10" width="14" aria-hidden="true">
+                  <use xlink:href="../icons.svg#icon-tick-16"></use>
+                </svg>
+              </div>
+            </div>
+          </div>
+        </span>
+
+        <hr />
+
+        <h2 id="menu-button-2E">Single-Select Menu Button with Icons and Text</h2>
+
+        <span class="menu-button menu-button-with-iconText">
           <button class="btn" type="button">
             <span class="btn__text">
               <svg class="icon icon--amex-18-colored" focusable="false" height="18" width="30" aria-hidden="true">
@@ -297,7 +351,7 @@ <h2 id="menu-button-4">Grouped Menu Button</h2>
           <button class="btn" aria-expanded="false" aria-haspopup="true" type="button">
             <span class="btn__cell">
               <span class="btn__text">Button</span>
-              <svg class="icon icon--chevron-down-12" focusable="false" height="10" width="14" aria-hidden="true">
+              <svg class="icon icon--chevron-down-12" focusable="false" height="10" width="14"  aria-label="AMEX">
                 <use xlink:href="../icons.svg#icon-chevron-down-12"></use>
               </svg>
             </span>
diff --git a/docs/makeup-menu-button/index.js b/docs/makeup-menu-button/index.js
index 45e7ff4b..a6e320e8 100644
--- a/docs/makeup-menu-button/index.js
+++ b/docs/makeup-menu-button/index.js
@@ -10,9 +10,14 @@ import "@ebay/skin/menu-button";
 // IMPORT
 import MenuButton from "../../packages/makeup-menu-button";
 
-window.onload = function () {
-  document.querySelectorAll(".menu-button").forEach(function (el, i) {
-    const widget = new MenuButton(el, el.classList.contains("menu-button-with-icon") && {valueTypeHTML: true});
+window.onload = function() {
+  document.querySelectorAll(".menu-button").forEach(function(el, i) {
+    const buttonValueType = el.classList.contains("menu-button-with-iconText")
+      ? "both"
+      : el.classList.contains("menu-button-with-icon")
+        ? "icon"
+        : "text";
+    const widget = new MenuButton(el, { buttonValueType });
 
     widget.menu.el.addEventListener("makeup-menu-select", (e) => console.log(e.type, e.detail));
     widget.menu.el.addEventListener("makeup-menu-change", (e) => console.log(e.type, e.detail));
diff --git a/packages/makeup-menu-button/dist/cjs/index.js b/packages/makeup-menu-button/dist/cjs/index.js
index 081d958b..71e54e03 100644
--- a/packages/makeup-menu-button/dist/cjs/index.js
+++ b/packages/makeup-menu-button/dist/cjs/index.js
@@ -12,9 +12,9 @@ const defaultOptions = {
   expandedClass: "menu-button--expanded",
   menuSelector: ".menu-button__menu",
   buttonTextSelector: ".btn__text",
-  valueSelector: ".menu-button__item-value",
-  // Selector to get value from
-  valueTypeHTML: false // If true, will get innerHTML of valueSelector, otherwise will get innerText
+  buttonValueType: "text",
+  // ["text", "icon", "both"],
+  iconSelector: ".icon"
 };
 class _default {
   constructor(widgetEl, selectedOptions) {
@@ -23,9 +23,7 @@ class _default {
     this.el = widgetEl;
     this._buttonEl = widgetEl.querySelector("button");
     this.menu = new _makeupMenu.default(widgetEl.querySelector(this._options.menuSelector), {
-      autoReset: "interactive",
-      valueSelector: this._options.valueSelector,
-      valueTypeHTML: this._options.valueTypeHTML
+      autoReset: "interactive"
     });
     this._buttonPrefix = (_this$_buttonEl$datas = this._buttonEl.dataset) === null || _this$_buttonEl$datas === void 0 ? void 0 : _this$_buttonEl$datas.makeupMenuButtonPrefix;
     this._buttonTextEl = this._buttonEl.querySelector(this._options.buttonTextSelector);
@@ -122,11 +120,22 @@ function _onMenuItemSelect(e) {
   if (e.detail.el.getAttribute("role") !== "menuitemradio") {
     return;
   }
-  const valueSelector = e.detail.el.querySelector(this._options.valueSelector);
-  if (valueSelector) {
-    const valueType = this._options.valueTypeHTML ? "innerHTML" : "innerText";
-    this._buttonTextEl[valueType] = valueSelector[valueType];
-  } else if (this._buttonPrefix) {
-    this._buttonTextEl.innerText = "".concat(this._buttonPrefix, " ").concat(e.detail.el.innerText);
+  const icon = e.detail.el.querySelector(this._options.iconSelector);
+  const text = e.detail.el.innerText.trim();
+  let content = this._buttonPrefix ? "".concat(this._buttonPrefix, " ").concat(text) : text;
+  if (icon) {
+    switch (this._options.buttonValueType) {
+      case "both":
+        content = "".concat(icon.outerHTML, " <span>").concat(content, "</span>");
+        break;
+      case "icon":
+        icon.setAttribute("aria-label", text);
+        icon.removeAttribute("aria-hidden");
+        content = icon.outerHTML;
+        break;
+      default:
+        break;
+    }
   }
+  this._buttonTextEl.innerHTML = content;
 }
diff --git a/packages/makeup-menu-button/dist/mjs/index.js b/packages/makeup-menu-button/dist/mjs/index.js
index bda6addd..4c550305 100644
--- a/packages/makeup-menu-button/dist/mjs/index.js
+++ b/packages/makeup-menu-button/dist/mjs/index.js
@@ -5,10 +5,9 @@ const defaultOptions = {
   expandedClass: "menu-button--expanded",
   menuSelector: ".menu-button__menu",
   buttonTextSelector: `.btn__text`,
-  valueSelector: ".menu-button__item-value",
-  // Selector to get value from
-  valueTypeHTML: false
-  // If true, will get innerHTML of valueSelector, otherwise will get innerText
+  buttonValueType: "text",
+  // ["text", "icon", "both"],
+  iconSelector: ".icon"
 };
 class src_default {
   constructor(widgetEl, selectedOptions) {
@@ -16,9 +15,7 @@ class src_default {
     this.el = widgetEl;
     this._buttonEl = widgetEl.querySelector("button");
     this.menu = new Menu(widgetEl.querySelector(this._options.menuSelector), {
-      autoReset: "interactive",
-      valueSelector: this._options.valueSelector,
-      valueTypeHTML: this._options.valueTypeHTML
+      autoReset: "interactive"
     });
     this._buttonPrefix = this._buttonEl.dataset?.makeupMenuButtonPrefix;
     this._buttonTextEl = this._buttonEl.querySelector(this._options.buttonTextSelector);
@@ -114,13 +111,24 @@ function _onMenuItemSelect(e) {
   if (e.detail.el.getAttribute("role") !== "menuitemradio") {
     return;
   }
-  const valueSelector = e.detail.el.querySelector(this._options.valueSelector);
-  if (valueSelector) {
-    const valueType = this._options.valueTypeHTML ? "innerHTML" : "innerText";
-    this._buttonTextEl[valueType] = valueSelector[valueType];
-  } else if (this._buttonPrefix) {
-    this._buttonTextEl.innerText = `${this._buttonPrefix} ${e.detail.el.innerText}`;
+  const icon = e.detail.el.querySelector(this._options.iconSelector);
+  const text = e.detail.el.innerText.trim();
+  let content = this._buttonPrefix ? `${this._buttonPrefix} ${text}` : text;
+  if (icon) {
+    switch (this._options.buttonValueType) {
+      case "both":
+        content = `${icon.outerHTML} <span>${content}</span>`;
+        break;
+      case "icon":
+        icon.setAttribute("aria-label", text);
+        icon.removeAttribute("aria-hidden");
+        content = icon.outerHTML;
+        break;
+      default:
+        break;
+    }
   }
+  this._buttonTextEl.innerHTML = content;
 }
 export {
   src_default as default
diff --git a/packages/makeup-menu-button/src/index.js b/packages/makeup-menu-button/src/index.js
index 35d76774..7093fb36 100644
--- a/packages/makeup-menu-button/src/index.js
+++ b/packages/makeup-menu-button/src/index.js
@@ -6,8 +6,8 @@ const defaultOptions = {
   expandedClass: "menu-button--expanded",
   menuSelector: ".menu-button__menu",
   buttonTextSelector: `.btn__text`,
-  valueSelector: ".menu-button__item-value", // Selector to get value from
-  valueTypeHTML: false, // If true, will get innerHTML of valueSelector, otherwise will get innerText
+  buttonValueType: "text", // ["text", "icon", "both"],
+  iconSelector: ".icon",
 };
 
 export default class {
@@ -17,8 +17,6 @@ export default class {
     this._buttonEl = widgetEl.querySelector("button");
     this.menu = new Menu(widgetEl.querySelector(this._options.menuSelector), {
       autoReset: "interactive",
-      valueSelector: this._options.valueSelector,
-      valueTypeHTML: this._options.valueTypeHTML,
     });
     this._buttonPrefix = this._buttonEl.dataset?.makeupMenuButtonPrefix;
     this._buttonTextEl = this._buttonEl.querySelector(this._options.buttonTextSelector);
@@ -132,11 +130,24 @@ function _onMenuItemSelect(e) {
     return;
   }
 
-  const valueSelector = e.detail.el.querySelector(this._options.valueSelector);
-  if (valueSelector) {
-    const valueType = this._options.valueTypeHTML ? "innerHTML" : "innerText";
-    this._buttonTextEl[valueType] = valueSelector[valueType];
-  } else if (this._buttonPrefix) {
-    this._buttonTextEl.innerText = `${this._buttonPrefix} ${e.detail.el.innerText}`;
+  const icon = e.detail.el.querySelector(this._options.iconSelector);
+  const text = e.detail.el.innerText.trim();
+  let content = this._buttonPrefix ? `${this._buttonPrefix} ${text}` : text;
+  
+  if (icon) {
+    switch (this._options.buttonValueType) {
+      case "both":
+        content = `${icon.outerHTML} <span>${content}</span>`;
+        break;
+      case "icon":
+        icon.setAttribute("aria-label", text);
+        icon.removeAttribute("aria-hidden");
+        content = icon.outerHTML;
+        break;
+      default:
+        break;
+    }
   }
+
+  this._buttonTextEl.innerHTML = content;
 }

From a3a54dc454c30ccebf842ae49fb1abaad3687147 Mon Sep 17 00:00:00 2001
From: Goutham Ponnada <sponnada@ebay.com>
Date: Fri, 26 Jan 2024 11:45:36 -0800
Subject: [PATCH 5/5] fix: set aria-haspopup to true for button

---
 packages/makeup-menu-button/dist/cjs/index.js | 1 +
 packages/makeup-menu-button/dist/mjs/index.js | 1 +
 packages/makeup-menu-button/src/index.js      | 1 +
 3 files changed, 3 insertions(+)

diff --git a/packages/makeup-menu-button/dist/cjs/index.js b/packages/makeup-menu-button/dist/cjs/index.js
index 71e54e03..e869bff1 100644
--- a/packages/makeup-menu-button/dist/cjs/index.js
+++ b/packages/makeup-menu-button/dist/cjs/index.js
@@ -22,6 +22,7 @@ class _default {
     this._options = Object.assign({}, defaultOptions, selectedOptions);
     this.el = widgetEl;
     this._buttonEl = widgetEl.querySelector("button");
+    this._buttonEl.setAttribute("aria-haspopup", "true");
     this.menu = new _makeupMenu.default(widgetEl.querySelector(this._options.menuSelector), {
       autoReset: "interactive"
     });
diff --git a/packages/makeup-menu-button/dist/mjs/index.js b/packages/makeup-menu-button/dist/mjs/index.js
index 4c550305..6d83376f 100644
--- a/packages/makeup-menu-button/dist/mjs/index.js
+++ b/packages/makeup-menu-button/dist/mjs/index.js
@@ -14,6 +14,7 @@ class src_default {
     this._options = Object.assign({}, defaultOptions, selectedOptions);
     this.el = widgetEl;
     this._buttonEl = widgetEl.querySelector("button");
+    this._buttonEl.setAttribute("aria-haspopup", "true");
     this.menu = new Menu(widgetEl.querySelector(this._options.menuSelector), {
       autoReset: "interactive"
     });
diff --git a/packages/makeup-menu-button/src/index.js b/packages/makeup-menu-button/src/index.js
index 7093fb36..1bbab9eb 100644
--- a/packages/makeup-menu-button/src/index.js
+++ b/packages/makeup-menu-button/src/index.js
@@ -15,6 +15,7 @@ export default class {
     this._options = Object.assign({}, defaultOptions, selectedOptions);
     this.el = widgetEl;
     this._buttonEl = widgetEl.querySelector("button");
+    this._buttonEl.setAttribute("aria-haspopup", "true");
     this.menu = new Menu(widgetEl.querySelector(this._options.menuSelector), {
       autoReset: "interactive",
     });