Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(combobox, stepper, table): respect user hidden attribute #10983

Open
wants to merge 26 commits into
base: dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
6dfa7c9
fix(combobox, stepper, table): respect user hidden attribute
josercarcamo Dec 4, 2024
cddcabd
Merge branch 'dev' into josercarcamo/8623-respect-user-hidden-attribu…
josercarcamo Dec 12, 2024
ff38449
Merged dev
josercarcamo Jan 8, 2025
e843d98
Fixed all e2e tests for combobox
josercarcamo Jan 8, 2025
e425670
Fixed all tests for table
josercarcamo Jan 8, 2025
1c3680c
Removed unnecessary change
josercarcamo Jan 8, 2025
4772e23
Changed hideItem to hiddenItem and fixed test cases
josercarcamo Jan 9, 2025
46686b4
Finished addressing all code review comments
josercarcamo Jan 9, 2025
dfb66ae
Added hidden-item mixin
josercarcamo Jan 9, 2025
759791a
Removed unnecessary test change
josercarcamo Jan 10, 2025
eee513d
Merged dev
josercarcamo Jan 13, 2025
3e38b65
Merge branch 'dev' into josercarcamo/8623-respect-user-hidden-attribu…
josercarcamo Jan 13, 2025
284d75c
Merge branch 'dev' into josercarcamo/8623-respect-user-hidden-attribu…
josercarcamo Jan 13, 2025
09af985
Merge branch 'dev' into josercarcamo/8623-respect-user-hidden-attribu…
josercarcamo Jan 17, 2025
c88ca71
Used html formatter
josercarcamo Jan 17, 2025
a90211f
Parameterized disabled/hidden
josercarcamo Jan 17, 2025
7d1f446
Changed step number to integer
josercarcamo Jan 17, 2025
f8a9016
Removed optional chaining operator; fixed combobox e2e error
josercarcamo Jan 17, 2025
37cabf8
Refactored statement in test per code review
josercarcamo Jan 18, 2025
3e146b9
Removed unnecessary test per review
josercarcamo Jan 18, 2025
1d4a2d3
Renamed argument to el
josercarcamo Jan 18, 2025
a46227d
Replaced conditional with use of isHidden
josercarcamo Jan 18, 2025
da25db2
Changed test to step through hidden element
josercarcamo Jan 18, 2025
ca3fedc
Fixed combobox test failure
josercarcamo Jan 20, 2025
a3ea10a
Fixed errors caused by hidden combobox item'
josercarcamo Jan 20, 2025
7da4daa
Merge branch 'dev' into josercarcamo/8623-respect-user-hidden-attribu…
josercarcamo Jan 20, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ Calcite follows [Conventional Commits](https://www.conventionalcommits.org/en/v1
Contributions should adhere to the `<type>(<scope>): <descriptive summary>` format and include the following:

- [Commit type](#commit-type)
- [Scope of change](#scope-of-change), _optional_
- [Scope of change](#scope-of-change), *optional*
- [Descriptive commit subject](#descriptive-commit-subject)

Check out the [contribution example](#contribution-example) for a formatted example, and explore [breaking change formatting](#breaking-changes) for consideration during Calcite's breaking change releases.
Expand All @@ -240,7 +240,7 @@ Contributions must adhere to **one** of the following types:

### Scope of change

_Optional_. Most contributions will include a scope, such as a component, multiple components, test(s), or utilities. For example:
*Optional*. Most contributions will include a scope, such as a component, multiple components, test(s), or utilities. For example:

- `text-area`
- `dropdown, dropdown-group, dropdown-item`
Expand Down
6 changes: 6 additions & 0 deletions packages/calcite-components/src/assets/styles/includes.scss
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,12 @@ $common-animatable-props: "background-color, block-size, border-color, box-shado
transition-timing-function: ease-in-out;
}

@mixin hidden-item() {
:host([hidden-item]) {
@apply hidden;
}
}

// Mixin for text highlighting styles.
// - this should be used in conjunction with the `text.tsx` util.
@mixin text-highlight-item() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,3 +48,4 @@
}

@include base-component();
@include hidden-item();
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,13 @@ export class ComboboxItemGroup extends LitElement {
*/
@property() scale: Scale = "m";

/**
* Specifies whether the user set the hidden attribute in the HTML
*
* @private
*/
@property({ reflect: true }) hiddenItem = false;

// #endregion

// #region Lifecycle
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,4 +150,5 @@ ul:focus {
line-height: var(--calcite-font-line-height-relative-snug);
}

@include hidden-item();
@include text-highlight-item();
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,13 @@ export class ComboboxItem extends LitElement implements InteractiveComponent {
*/
@property() value: any;

/**
* Specifies whether the user set the hidden attribute in the HTML
*
* @private
*/
@property({ reflect: true }) hiddenItem = false;

// #endregion

// #region Events
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -469,7 +469,7 @@ describe("calcite-combobox", () => {
await page.waitForTimeout(DEBOUNCE.filter);

const visibleItemsAndGroups = await page.findAll(
"calcite-combobox-item:not([hidden]), calcite-combobox-item-group:not([hidden])",
"calcite-combobox-item:not([hidden-item]), calcite-combobox-item-group:not([hidden-item])",
);
const visibleItemAndGroupIds = await Promise.all(visibleItemsAndGroups.map((item) => item.getProperty("id")));

Expand Down Expand Up @@ -497,7 +497,7 @@ describe("calcite-combobox", () => {
await page.waitForTimeout(DEBOUNCE.filter);

const filteredItemsAndGroups = await page.findAll(
"calcite-combobox-item:not([hidden]), calcite-combobox-item-group:not([hidden])",
"calcite-combobox-item:not([hidden-item]), calcite-combobox-item-group:not([hidden-item])",
);
const filteredItemAndGroupIds = await Promise.all(filteredItemsAndGroups.map((item) => item.getProperty("id")));

Expand All @@ -518,7 +518,7 @@ describe("calcite-combobox", () => {
await page.waitForTimeout(DEBOUNCE.filter);

const allVisibleItemAndGroups = await page.findAll(
"calcite-combobox-item:not([hidden]), calcite-combobox-item-group:not([hidden])",
"calcite-combobox-item:not([hidden]):not([hidden-item]), calcite-combobox-item-group:not([hidden]):not([hidden-item])",
);
const allVisibleItemAndGroupIds = await Promise.all(
allVisibleItemAndGroups.map((item) => item.getProperty("id")),
Expand Down Expand Up @@ -570,7 +570,7 @@ describe("calcite-combobox", () => {
await page.waitForChanges();
await page.waitForTimeout(DEBOUNCE.filter);

const visibleItems = await page.findAll("calcite-combobox-item:not([hidden])");
const visibleItems = await page.findAll("calcite-combobox-item:not([hidden-item])");

expect(visibleItems.length).toBe(1);
expect(await visibleItems[0].getProperty("value")).toBe("1");
Expand Down Expand Up @@ -725,7 +725,7 @@ describe("calcite-combobox", () => {

expect(await combobox.getProperty("filteredItems")).toHaveLength(2);

const visibleItems = await page.findAll("calcite-combobox-item:not([hidden])");
const visibleItems = await page.findAll("calcite-combobox-item:not([hidden]):not([hidden-item])");

expect(visibleItems.map((item) => item.id)).toEqual(["text-label-match", "description-match"]);
});
Expand Down Expand Up @@ -919,6 +919,56 @@ describe("calcite-combobox", () => {
}
});

it("should show correct number of items when child hidden", async () => {
const page = await newE2EPage();
await page.setContent(html`
<calcite-combobox label="custom values" allow-custom-values placeholder="placeholder" max-items="6">
<calcite-combobox-item value="Trees" text-label="Trees" selected>
<calcite-combobox-item value="Pine" text-label="Pine">
<calcite-combobox-item value="Pine Nested" text-label="Pine Nested"></calcite-combobox-item>
</calcite-combobox-item>
<calcite-combobox-item value="Sequoia" hidden text-label="Sequoia"></calcite-combobox-item>
<calcite-combobox-item value="Douglas Fir" text-label="Douglas Fir"></calcite-combobox-item>
</calcite-combobox-item>
<calcite-combobox-item value="Rocks" text-label="Rocks"></calcite-combobox-item>
</calcite-combobox>
`);
await page.waitForChanges();

const element = await page.find("calcite-combobox");
await element.click();
await page.waitForChanges();
const items = await page.findAll("calcite-combobox-item, calcite-combobox-item-group");
for (let i = 0; i < items.length; i++) {
expect(await items[i].isIntersectingViewport()).toBe(i !== 3);
}
});

it("should show correct number of items when parent hidden", async () => {
const page = await newE2EPage();
await page.setContent(html`
<calcite-combobox label="custom values" allow-custom-values placeholder="placeholder" max-items="6">
<calcite-combobox-item value="Trees" text-label="Trees" hidden>
<calcite-combobox-item value="Pine" text-label="Pine">
<calcite-combobox-item value="Pine Nested" text-label="Pine Nested"></calcite-combobox-item>
</calcite-combobox-item>
<calcite-combobox-item value="Sequoia" disabled text-label="Sequoia"></calcite-combobox-item>
<calcite-combobox-item value="Douglas Fir" text-label="Douglas Fir"></calcite-combobox-item>
</calcite-combobox-item>
<calcite-combobox-item value="Rocks" text-label="Rocks"></calcite-combobox-item>
</calcite-combobox>
`);
await page.waitForChanges();

const element = await page.find("calcite-combobox");
await element.click();
await page.waitForChanges();
const items = await page.findAll("calcite-combobox-item, calcite-combobox-item-group");
for (let i = 0; i < items.length; i++) {
expect(await items[i].isIntersectingViewport()).toBe(i === 5);
}
});

it("should show correct max items after selection", async () => {
const page = await newE2EPage();
const maxItems = 6;
Expand Down
13 changes: 7 additions & 6 deletions packages/calcite-components/src/components/combobox/combobox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ import type { Chip } from "../chip/chip";
import type { ComboboxItemGroup as HTMLCalciteComboboxItemGroupElement } from "../combobox-item-group/combobox-item-group";
import type { ComboboxItem as HTMLCalciteComboboxItemElement } from "../combobox-item/combobox-item";
import type { Label } from "../label/label";
import { isHidden } from "../../utils/component";
import T9nStrings from "./assets/t9n/messages.en.json";
import { ComboboxChildElement, GroupData, ItemData, SelectionDisplay } from "./interfaces";
import { ComboboxItemGroupSelector, ComboboxItemSelector, CSS, IDS } from "./resources";
Expand Down Expand Up @@ -145,20 +146,20 @@ export class Combobox

itemsAndGroups.forEach((item) => {
if (matchAll) {
item.hidden = false;
item.hiddenItem = false;
return;
}

const hidden = !find(item, filteredData);
item.hidden = hidden;
item.hiddenItem = hidden;
const [parent, grandparent] = item.ancestors;

if (find(parent, filteredData) || find(grandparent, filteredData)) {
item.hidden = false;
item.hiddenItem = false;
}

if (!hidden) {
item.ancestors.forEach((ancestor) => (ancestor.hidden = false));
item.ancestors.forEach((ancestor) => (ancestor.hiddenItem = false));
}
});

Expand Down Expand Up @@ -1126,7 +1127,7 @@ export class Combobox

private getMaxScrollerHeight(): number {
const allItemsAndGroups = [...this.groupItems, ...this.getItems(true)];
const items = allItemsAndGroups.filter((item) => !item.hidden);
const items = allItemsAndGroups.filter((item) => !isHidden(item));

const { maxItems } = this;

Expand Down Expand Up @@ -1226,7 +1227,7 @@ export class Combobox
}

private getFilteredItems(): HTMLCalciteComboboxItemElement["el"][] {
return this.filterText === "" ? this.items : this.items.filter((item) => !item.hidden);
return this.filterText === "" ? this.items : this.items.filter((item) => !isHidden(item));
}

private updateItems = debounce((): void => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ describe("calcite-input-time-zone", () => {
}

let matchedTimeZoneItems = await page.findAll(
"calcite-input-time-zone >>> calcite-combobox-item:not([hidden])",
"calcite-input-time-zone >>> calcite-combobox-item:not([hidden]):not([hidden-item])",
);
expect(matchedTimeZoneItems.length).toBeGreaterThan(1);

Expand All @@ -234,7 +234,9 @@ describe("calcite-input-time-zone", () => {
await page.waitForChanges();
await page.waitForTimeout(DEBOUNCE.filter);

matchedTimeZoneItems = await page.findAll("calcite-input-time-zone >>> calcite-combobox-item:not([hidden])");
matchedTimeZoneItems = await page.findAll(
"calcite-input-time-zone >>> calcite-combobox-item:not([hidden]):not([hidden-item])",
);

expect(matchedTimeZoneItems).toHaveLength(1);

Expand All @@ -243,7 +245,9 @@ describe("calcite-input-time-zone", () => {
await page.waitForChanges();
await page.waitForTimeout(DEBOUNCE.filter);

matchedTimeZoneItems = await page.findAll("calcite-input-time-zone >>> calcite-combobox-item:not([hidden])");
matchedTimeZoneItems = await page.findAll(
"calcite-input-time-zone >>> calcite-combobox-item:not([hidden]):not([hidden-item])",
);

expect(matchedTimeZoneItems).toHaveLength(1);

Expand All @@ -252,15 +256,19 @@ describe("calcite-input-time-zone", () => {
await page.waitForChanges();
await page.waitForTimeout(DEBOUNCE.filter);

matchedTimeZoneItems = await page.findAll("calcite-input-time-zone >>> calcite-combobox-item:not([hidden])");
matchedTimeZoneItems = await page.findAll(
"calcite-input-time-zone >>> calcite-combobox-item:not([hidden]):not([hidden-item])",
);

expect(matchedTimeZoneItems).toHaveLength(2);

await clearSearchTerm(searchTerms[1]);
await page.waitForChanges();
await page.waitForTimeout(DEBOUNCE.filter);

matchedTimeZoneItems = await page.findAll("calcite-input-time-zone >>> calcite-combobox-item:not([hidden])");
matchedTimeZoneItems = await page.findAll(
"calcite-input-time-zone >>> calcite-combobox-item:not([hidden]):not([hidden-item])",
);

expect(matchedTimeZoneItems.length).toBeGreaterThan(1);
});
Expand Down Expand Up @@ -545,7 +553,7 @@ describe("calcite-input-time-zone", () => {
await page.waitForTimeout(DEBOUNCE.filter);

const sharedOffsetTimeZoneItems = await page.findAll(
"calcite-input-time-zone >>> calcite-combobox-item:not([hidden])",
"calcite-input-time-zone >>> calcite-combobox-item:not([hidden]):not([hidden-item])",
);
expect(sharedOffsetTimeZoneItems).toHaveLength(2);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -407,3 +407,4 @@
}

@include base-component();
@include hidden-item();
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import {
import { IconNameOrString } from "../icon/interfaces";
import { useT9n } from "../../controllers/useT9n";
import type { Stepper } from "../stepper/stepper";
import { isHidden } from "../../utils/component";
import { CSS } from "./resources";
import T9nStrings from "./assets/t9n/messages.en.json";
import { styles } from "./stepper-item.scss";
Expand Down Expand Up @@ -92,6 +93,13 @@ export class StepperItem extends LitElement implements InteractiveComponent, Loa
/** When `true`, the icon will be flipped when the element direction is right-to-left (`"rtl"`). */
@property({ reflect: true }) iconFlipRtl = false;

/**
* When `true`, the item will be hidden
*
* @private
* */
@property({ reflect: true }) hiddenItem = false;

/**
* Specifies the layout of the `calcite-stepper-item` inherited from parent `calcite-stepper`, defaults to `horizontal`.
*
Expand Down Expand Up @@ -274,6 +282,7 @@ export class StepperItem extends LitElement implements InteractiveComponent, Loa
private handleItemClick(event: MouseEvent): void {
if (
this.disabled ||
isHidden(this.el) ||
(this.layout === "horizontal" &&
event
.composedPath()
Expand Down Expand Up @@ -303,9 +312,11 @@ export class StepperItem extends LitElement implements InteractiveComponent, Loa
}

private getItemPosition(): number {
return Array.from(this.parentStepperEl?.querySelectorAll("calcite-stepper-item")).indexOf(
this.el,
);
return Array.from(
this.parentStepperEl?.querySelectorAll(
"calcite-stepper-item:not([hidden]):not([hidden-item])",
),
).indexOf(this.el);
}

// #endregion
Expand Down
Loading
Loading