Skip to content

Commit

Permalink
fix: do not render hidden combo box items (#8181) (#8183)
Browse files Browse the repository at this point in the history
Co-authored-by: Sascha Ißbrücker <sissbruecker@vaadin.com>
  • Loading branch information
vaadin-bot and sissbruecker authored Nov 19, 2024
1 parent faf217c commit 5c7ac54
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 1 deletion.
2 changes: 1 addition & 1 deletion packages/combo-box/src/vaadin-combo-box-item-mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ export const ComboBoxItemMixin = (superClass) =>
* It is not guaranteed that the update happens immediately (synchronously) after it is requested.
*/
requestContentUpdate() {
if (!this.renderer) {
if (!this.renderer || this.hidden) {
return;
}

Expand Down
32 changes: 32 additions & 0 deletions packages/combo-box/test/item-renderer.common.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,38 @@ describe('item renderer', () => {
expect(comboBox.renderer.callCount).to.be.equal(comboBox.items.length * 2);
});

it('should not run renderers for invisible items', () => {
// Set up an item renderer that maps item data to DOM elements.
const itemContents = {
foo: document.createElement('div'),
bar: document.createElement('div'),
baz: document.createElement('div'),
};
comboBox.renderer = (root, _, { item }) => {
root.textContent = '';
root.appendChild(itemContents[item]);
};
comboBox.opened = true;

// Filter the items
// This renders `bar` into the first item now, and hides the other items.
// However, the second item still has `bar` as item data.
setInputValue(comboBox, 'bar');

const filteredItem = getAllItems(comboBox)[0];
expect(filteredItem.children.length).to.equal(1);
expect(filteredItem.children[0]).to.equal(itemContents.bar);

// Now run requestContentUpdate. This should only render the first item, but
// not the second one. We test this by verifying that the `bar` item content
// was not moved to the second item by its renderer.
comboBox.requestContentUpdate();

const allItems = getAllItems(comboBox);
expect(allItems[0].children.length).to.equal(1);
expect(allItems[0].children[0]).to.equal(itemContents.bar);
});

it('should not throw if requestContentUpdate() called before opening', () => {
expect(() => comboBox.requestContentUpdate()).not.to.throw(Error);
});
Expand Down

0 comments on commit 5c7ac54

Please sign in to comment.