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

[ch-dialog][ch-popover] Rename property name hidden to show #469

Merged
merged 32 commits into from
Jan 7, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
5767e11
[ch-dialog] Rename `hidden` prop to `show`
bsastregx Jan 2, 2025
0bdb335
[ch-dialog] Update readme
bsastregx Jan 2, 2025
a6e3187
[ch-dialog] Include base tests
bsastregx Jan 2, 2025
10b6315
[ch-dialog] Include behavior tests files
bsastregx Jan 2, 2025
66a3979
[ch-dialog] Update `getDialogPartRect` helper function
bsastregx Jan 2, 2025
8dead16
[ch-dialog] Include "resize" behavior test (WIP)
bsastregx Jan 2, 2025
81d84da
[ch-dialog] Remove tests
bsastregx Jan 2, 2025
02f9f2f
Merge branch 'main' into refactor/rename-hidden-to-show
bsastregx Jan 2, 2025
6c083cb
[ch-popover] Update `hidden` prop name, in favor of `show`
bsastregx Jan 2, 2025
ac7d50b
[ch-tooltip] Update `ch-popover` `hidden` property name
bsastregx Jan 2, 2025
025c19e
[ch-combo-box] Update `ch-popover` `hidden` property name
bsastregx Jan 2, 2025
1c64f84
[ch-dropdown] Update `ch-popover` `hidden` property name
bsastregx Jan 2, 2025
477eab2
Update readme.md
bsastregx Jan 2, 2025
7f47eae
[combo-box] Apply PR review suggestions
bsastregx Jan 3, 2025
cbcc818
`[ch-dialog]` Apply PR review suggestions
bsastregx Jan 3, 2025
452f776
`[ch-popover]` Apply PR review suggestions
bsastregx Jan 3, 2025
9e81a8f
`[ch-tooltip]` Apply PR review suggestions
bsastregx Jan 3, 2025
a9f56ed
`[ch-dialog]` Include some semantic validation
bsastregx Jan 3, 2025
ac77219
`[ch-dialog]` improve `#setBorderSizeWatcher` call on `componentWillR…
bsastregx Jan 3, 2025
4c62eb5
`[ch-dialog]`Rename tests filename
bsastregx Jan 3, 2025
779ad2c
`[ch-dialog]` Apply PR review suggestions
bsastregx Jan 3, 2025
2f11b4b
`[ch-dialog]` Split `contstraints.e2e.ts` into two files
bsastregx Jan 3, 2025
f4b8525
`[ch-dialog]` Fix failing test on `[basic]`
bsastregx Jan 3, 2025
afefe13
`[ch-dialog][ch-popover]` Apply PR review suggestions
bsastregx Jan 6, 2025
0c94cdc
`[ch-dialog]` Force push
bsastregx Jan 7, 2025
68d2f3a
`[ch-dialog]` assert parts not null on `[basic]` test
bsastregx Jan 7, 2025
4da7a28
`[ch-dialog]` Comment `testDefaultProperty` to debug
bsastregx Jan 7, 2025
6ee6a3b
`[ch-dialog]` Comment `testDefaultProperty` function to debug
bsastregx Jan 7, 2025
ccffbf2
`[ch-dialog]` Include `await` on tests that were failing
bsastregx Jan 7, 2025
c5d91a1
`[ch-dialog]` Rename `chDialogRef` in favor of `dialogRef`
bsastregx Jan 7, 2025
1fe3abd
`[ch-dialog]` Set property `show` to true on some tests
bsastregx Jan 7, 2025
c08aef7
`[ch-dialog]` Remove unrequired `not.toBeNull()`
bsastregx Jan 7, 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
2 changes: 1 addition & 1 deletion src/components/combo-box/combo-box.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -972,7 +972,7 @@ export class ChComboBoxRender
blockAlign="outside-end"
inlineAlign={this.popoverInlineAlign}
closeOnClickOutside
hidden={false}
show
popover="manual"
resizable={this.resizable}
inlineSizeMatch="action-element-as-minimum"
Expand Down
4 changes: 4 additions & 0 deletions src/components/dialog/dialog.scss
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,10 @@ $ch-dialog-y--same-layer: calc(
display: contents;
}

:host(.ch-dialog--hidden) {
display: none;
}

// - - - - - - - - - - - - - - - -
// Header
// - - - - - - - - - - - - - - - -
Expand Down
34 changes: 16 additions & 18 deletions src/components/dialog/dialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -268,21 +268,20 @@ export class ChDialog {
@Prop() readonly closeButtonAccessibleName?: string;

/**
* Specifies whether the dialog is hidden or visible.
* Specifies whether the dialog is shown or not.
*/
// eslint-disable-next-line @stencil-community/ban-default-true
@Prop({ mutable: true, reflect: true }) hidden = true;
@Watch("hidden")
handleHiddenChange(hidden: boolean) {
@Prop({ mutable: true }) show: boolean = false;
@Watch("show")
showChanged(show: boolean) {
// Schedule update for watchers
this.#checkBorderSizeWatcher = true;
this.#checkPositionWatcher = true;

// Update the dialog visualization
if (hidden) {
this.#dialogRef.close();
} else {
if (show) {
this.#showModal();
} else {
this.#dialogRef.close();
}
}

Expand All @@ -291,7 +290,7 @@ export class ChDialog {
* interrupt interaction with the rest of the page being inert, while
* non-modal dialog boxes allow interaction with the rest of the page.
*
* Note: If `hidden !== false`, this property does not reflect changes on
* Note: If `show !== true`, this property does not reflect changes on
* runtime, since at the time of writing browsers do not support switching
* from modal to not-modal, (or vice-versa).
*/
Expand Down Expand Up @@ -351,14 +350,12 @@ export class ChDialog {
this.#checkBorderSizeWatcher = false;

// Wait until the resize edges have been rendered
requestAnimationFrame(() => {
this.#setBorderSizeWatcher();
});
requestAnimationFrame(() => setTimeout(this.#setBorderSizeWatcher));
}
}

componentDidLoad() {
if (!this.hidden) {
if (this.show) {
// Schedule update for watchers
this.#checkBorderSizeWatcher = true;
this.#checkPositionWatcher = true;
Expand Down Expand Up @@ -398,7 +395,7 @@ export class ChDialog {
};

#handleDialogClose = () => {
this.hidden = true;
this.show = false;
// Emit events only when the action is committed by the user
this.dialogClosed.emit();
document.removeEventListener("click", this.#evaluateClickOnDocument, {
Expand Down Expand Up @@ -505,7 +502,7 @@ export class ChDialog {
};

#closeHandler = () => {
this.hidden = true;
this.show = false;
};

#evaluateClickOnDocument = (e: MouseEvent) => {
Expand Down Expand Up @@ -653,7 +650,7 @@ export class ChDialog {
*/
// eslint-disable-next-line @stencil-community/own-props-must-be-private
#setBorderSizeWatcher = () => {
if (!this.resizable || this.hidden) {
if (!this.resizable || !this.show) {
this.#removeBorderSizeWatcher();
return;
}
Expand Down Expand Up @@ -734,9 +731,10 @@ export class ChDialog {
return (
<Host
class={{
"gx-dialog-header-drag": !this.hidden && this.allowDrag === "header",
"gx-dialog-header-drag": this.show && this.allowDrag === "header",
"ch-dialog--modal": this.modal,
"ch-dialog--non-modal": !this.modal,
"ch-dialog--hidden": !this.show,
[RESIZING_CLASS]: this.resizing
}}
>
Expand Down Expand Up @@ -783,7 +781,7 @@ export class ChDialog {
)}

{this.resizable &&
!this.hidden && [
this.show && [
<div
key="edge-block-start"
class="edge__block-start"
Expand Down
22 changes: 11 additions & 11 deletions src/components/dialog/readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,17 @@ interactive component.

## Properties

| Property | Attribute | Description | Type | Default |
| --------------------------- | ------------------------------ | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | --------------------------- | ----------- |
| `adjustPositionAfterResize` | `adjust-position-after-resize` | `true` if the dialog should be repositioned after resize. | `boolean` | `false` |
| `allowDrag` | `allow-drag` | "box" will allow the dialog to be draggable from both the header and the content. "header" will allow the dialog to be draggable only from the header. "no" disables dragging completely. | `"box" \| "header" \| "no"` | `"no"` |
| `caption` | `caption` | Refers to the dialog title. I will ve visible if 'showHeader´is true. | `string` | `undefined` |
| `closeButtonAccessibleName` | `close-button-accessible-name` | Specifies a short string, typically 1 to 3 words, that authors associate with an element to provide users of assistive technologies with a label for the element. This label is used for the close button of the header. | `string` | `undefined` |
| `hidden` | `hidden` | Specifies whether the dialog is hidden or visible. | `boolean` | `true` |
| `modal` | `modal` | Specifies whether the dialog is a modal or not. Modal dialog boxes interrupt interaction with the rest of the page being inert, while non-modal dialog boxes allow interaction with the rest of the page. Note: If `hidden !== false`, this property does not reflect changes on runtime, since at the time of writing browsers do not support switching from modal to not-modal, (or vice-versa). | `boolean` | `true` |
| `resizable` | `resizable` | Specifies whether the control can be resized. If `true` the control can be resized at runtime by dragging the edges or corners. | `boolean` | `false` |
| `showFooter` | `show-footer` | Specifies whether the dialog footer is hidden or visible. | `boolean` | `false` |
| `showHeader` | `show-header` | Specifies whether the dialog header is hidden or visible. | `boolean` | `false` |
| Property | Attribute | Description | Type | Default |
| --------------------------- | ------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | --------------------------- | ----------- |
| `adjustPositionAfterResize` | `adjust-position-after-resize` | `true` if the dialog should be repositioned after resize. | `boolean` | `false` |
| `allowDrag` | `allow-drag` | "box" will allow the dialog to be draggable from both the header and the content. "header" will allow the dialog to be draggable only from the header. "no" disables dragging completely. | `"box" \| "header" \| "no"` | `"no"` |
| `caption` | `caption` | Refers to the dialog title. I will ve visible if 'showHeader´is true. | `string` | `undefined` |
| `closeButtonAccessibleName` | `close-button-accessible-name` | Specifies a short string, typically 1 to 3 words, that authors associate with an element to provide users of assistive technologies with a label for the element. This label is used for the close button of the header. | `string` | `undefined` |
| `modal` | `modal` | Specifies whether the dialog is a modal or not. Modal dialog boxes interrupt interaction with the rest of the page being inert, while non-modal dialog boxes allow interaction with the rest of the page. Note: If `show !== true`, this property does not reflect changes on runtime, since at the time of writing browsers do not support switching from modal to not-modal, (or vice-versa). | `boolean` | `true` |
| `resizable` | `resizable` | Specifies whether the control can be resized. If `true` the control can be resized at runtime by dragging the edges or corners. | `boolean` | `false` |
| `show` | `show` | Specifies whether the dialog is shown or not. | `boolean` | `false` |
| `showFooter` | `show-footer` | Specifies whether the dialog footer is hidden or visible. | `boolean` | `false` |
| `showHeader` | `show-header` | Specifies whether the dialog header is hidden or visible. | `boolean` | `false` |


## Events
Expand Down
41 changes: 41 additions & 0 deletions src/components/dialog/tests/accessibility.e2e.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import { E2EElement, E2EPage, newE2EPage } from "@stencil/core/testing";

describe("[ch-dialog][accessibility]", () => {
let page: E2EPage;
let dialogRef: E2EElement;

beforeEach(async () => {
page = await newE2EPage({
html: `<ch-dialog></ch-dialog>`,
failOnConsoleError: true
});

dialogRef = await page.find("ch-dialog");

// Set Properties
dialogRef.setProperty("showHeader", true);
dialogRef.setProperty("showFooter", true);
dialogRef.setProperty("show", true);
await page.waitForChanges();
});

it("should not render a header element", async () => {
const headerEl = await page.find("ch-dialog >>> header");
expect(headerEl).toBeFalsy();
});

it("should not render a footer element", async () => {
const footerEl = await page.find("ch-dialog >>> footer");
expect(footerEl).toBeFalsy();
});

it("should not include a role attribute on the part='header' element", async () => {
const headerPartRef = await page.find("ch-dialog >>> [part='header']");
expect(headerPartRef).not.toHaveAttribute("role");
});

it("should not include a role attribute on the part='footer' element", async () => {
const footerPartRef = await page.find("ch-dialog >>> [part='footer']");
expect(footerPartRef).not.toHaveAttribute("role");
});
});
99 changes: 99 additions & 0 deletions src/components/dialog/tests/basic.e2e.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
import { E2EElement, E2EPage, newE2EPage } from "@stencil/core/testing";

describe("[ch-dialog][basic]", () => {
let page: E2EPage;
let dialogRef: E2EElement;

const testDefaultProperty = async (
propertyName: string,
expectedValue: any
) => {
it(`the "${propertyName}" property should be ${
expectedValue === undefined ? "undefined" : `"${expectedValue}"`
}`, async () => {
const propertyValue = await dialogRef.getProperty(propertyName);
if (expectedValue === undefined) {
expect(propertyValue).toBeUndefined();
} else {
expect(propertyValue).toBe(expectedValue);
}
});
};

beforeEach(async () => {
page = await newE2EPage({
html: `<ch-dialog></ch-dialog>`,
failOnConsoleError: true
});

dialogRef = await page.find("ch-dialog");
});

// Validate shadowRoot

it("should have a shadowRoot", async () => {
expect(dialogRef.shadowRoot).toBeTruthy();
});

// Validate properties default values

testDefaultProperty("adjustPositionAfterResize", false);

testDefaultProperty("allowDrag", "no");

testDefaultProperty("caption", undefined);

testDefaultProperty("closeButtonAccessibleName", undefined);

testDefaultProperty("show", false);

testDefaultProperty("modal", true);

testDefaultProperty("resizable", false);

testDefaultProperty("showFooter", false);

testDefaultProperty("showHeader", false);

// Validate id's

it("should not include an id on any of the resize-bar elements", async () => {
dialogRef.setProperty("resizable", true);
dialogRef.setProperty("show", true);
await page.waitForChanges();

const partsNames = [
"edge edge-block-start",
"edge edge-inline-end",
"edge edge-block-end",
"edge edge-inline-start",
"corner corner-block-start-inline-start",
"corner corner-block-start-inline-end",
"corner corner-block-end-inline-start",
"corner corner-block-end-inline-end"
];

for (const part of partsNames) {
const resizePartRef = await page.find(`ch-dialog >>> [part="${part}"]`);
expect(resizePartRef).not.toHaveAttribute("id");
}
});

it("should not include an id on the part='content' element", async () => {
dialogRef.setProperty("show", true);
await page.waitForChanges();
const contentPartRef = await page.find("ch-dialog >>> [part='content']");
expect(contentPartRef).not.toHaveAttribute("id");
});

it("should not include an id on the part='close-button' element", async () => {
dialogRef.setProperty("showHeader", true);
dialogRef.setProperty("show", true);
await page.waitForChanges();

const closeButtonPartRef = await page.find(
"ch-dialog >>> [part='close-button']"
);
expect(closeButtonPartRef).not.toHaveAttribute("id");
});
});
2 changes: 1 addition & 1 deletion src/components/dropdown/internal/dropdown/dropdown.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -473,7 +473,7 @@ export class ChDropDown implements ChComponent {
actionElement={this.#mainAction as HTMLButtonElement}
firstLayer={this.level === -1 || this.actionGroupParent}
popover="manual"
hidden={!this.expanded}
show={this.expanded}
inlineAlign={xAlignMapping}
blockAlign={yAlignMapping}
>
Expand Down
Loading
Loading