Merge pull request #796 from Trendyol/fix/793-select

fix(select): fix the issue where the popover for a disabled select can still open using keydown events
Trendyol · Feb 20, 2024 · ad09502 · ad09502
2 parents c3e664c + 5cf4a4c
commit ad09502
Show file tree

Hide file tree

Showing 3 changed files with 35 additions and 5 deletions.
diff --git a/src/components/select/bl-select.css b/src/components/select/bl-select.css
@@ -162,6 +162,10 @@
   text-overflow: ellipsis;
 }
 
+:host([disabled]) .select-input .selected-options {
+  color: var(--bl-color-neutral-light);
+}
+
 .selected-options li {
   display: inline;
   font-size: var(--font-size);
@@ -176,6 +180,10 @@
   display: none;
 }
 
+:host([disabled]) .additional-selection-count {
+  color: var(--bl-color-neutral-light);
+}
+
 :host([disabled]) .selected-options li {
   color: var(--bl-color-neutral-light);
 }

diff --git a/src/components/select/bl-select.test.ts b/src/components/select/bl-select.test.ts
@@ -596,6 +596,25 @@ describe("bl-select", () => {
       expect(document.activeElement).to.not.equal(blSelect);
     });
 
+
+    it("should not open popover if it is disabled", async () => {
+      // if it is disabled, it should not open popover and return from function
+      blSelect.disabled = true;
+
+      // given
+
+      await sendKeys({
+        press: tabKey,
+      });
+
+      await sendKeys({
+        press: "Space",
+      });
+
+      // then
+      expect(blSelect.opened).to.equal(false);
+    });
+
     ["Space", "Enter", "ArrowDown", "ArrowUp"].forEach(keyCode => {
       it(`should open popover with ${keyCode} key`, async () => {
         //given

diff --git a/src/components/select/bl-select.ts b/src/components/select/bl-select.ts
@@ -152,7 +152,7 @@ export default class BlSelect<ValueType extends FormValue = string> extends Form
   /**
    * Views select all option in multiple select
    */
-  @property({ type: Boolean, attribute: "view-select-all" })
+  @property({ type: Boolean, attribute: "view-select-all", converter: stringBooleanConverter() })
   viewSelectAll = false;
 
   /**
@@ -439,12 +439,15 @@ export default class BlSelect<ValueType extends FormValue = string> extends Form
           >`
         : ""}
 
-      <div class="actions" @click=${this._togglePopover}>
+      <div class="actions">
         ${this.opened ? (this.searchBarLoadingState ? searchLoadingIcon : searchMagIcon) : ""}
         ${!this.opened ? removeButton : ""} ${actionDivider}
-        <bl-icon class="dropdown-icon open" name="arrow_up"></bl-icon>
 
-        <bl-icon class="dropdown-icon closed" name="arrow_down"></bl-icon>
+        <div @click=${this._togglePopover}>
+          <bl-icon class="dropdown-icon open" name="arrow_up"></bl-icon>
+
+          <bl-icon class="dropdown-icon closed" name="arrow_down"></bl-icon>
+        </div>
       </div>
     </fieldset>`;
 
@@ -455,7 +458,7 @@ export default class BlSelect<ValueType extends FormValue = string> extends Form
             "select-input": true,
             "has-overflowed-options": this._additionalSelectedOptionCount > 0,
           })}
-          tabindex="${this.disabled ? "-1" : 0}"
+          tabindex="${ifDefined(this.disabled ? undefined : 0)}"
           ?autofocus=${this.autofocus}
           @click=${this._togglePopover}
           role="button"