Skip to content

Commit

Permalink
[ch-dialog][ch-popover] Rename property name hidden to show (#469)
Browse files Browse the repository at this point in the history
* [ch-dialog] Rename `hidden` prop to `show`

* [ch-dialog] Update readme

* [ch-dialog] Include base tests

Include the following base tests files:
- basic
- parts
- slots

* [ch-dialog] Include behavior tests files

add the following tests files:
- drag
- modal
- position

* [ch-dialog] Update `getDialogPartRect` helper function

Improve error log

* [ch-dialog] Include "resize" behavior test (WIP)

* [ch-dialog] Remove tests

Tests will be added on another PR

* [ch-popover] Update `hidden` prop name, in favor of `show`

Update property and showcase file accordingly

* [ch-tooltip] Update `ch-popover` `hidden` property name

* [ch-combo-box] Update `ch-popover` `hidden` property name

* [ch-dropdown] Update `ch-popover` `hidden` property name

* Update readme.md

* [combo-box] Apply PR review suggestions

* `[ch-dialog]` Apply PR review suggestions

* `[ch-popover]` Apply PR review suggestions

* `[ch-tooltip]` Apply PR review suggestions

* `[ch-dialog]` Include some semantic validation

* `[ch-dialog]` improve `#setBorderSizeWatcher` call on `componentWillRender`

- Add a `setTimeout` in order to force execution after browser paint. This fixes some E2E tests random fails, due to `this.#resizeLayer` being `undefined`.

* `[ch-dialog]`Rename tests filename

* `[ch-dialog]` Apply PR review suggestions

* `[ch-dialog]` Split `contstraints.e2e.ts` into two files

* `[ch-dialog]` Fix failing test on `[basic]`

For the button to be rendered, `showHeader` property should be `true`

* `[ch-dialog][ch-popover]` Apply PR review suggestions

* `[ch-dialog]` Force push

* `[ch-dialog]` assert parts not null on `[basic]` test

* `[ch-dialog]` Comment `testDefaultProperty` to debug

Some tests are failing on `basic.e2e.ts` only on the server. Check what happens if `testDefaultProperty` is not run.

* `[ch-dialog]` Comment `testDefaultProperty` function to debug

* `[ch-dialog]` Include `await` on tests that were failing

* `[ch-dialog]` Rename `chDialogRef` in favor of `dialogRef`

* `[ch-dialog]` Set property `show` to true on some tests

Although they are not required at the time of writting, setting this property to true ensures future compatibility, in the case the parts involved on these tests are dependent on the `show` property to be rendered.

* `[ch-dialog]` Remove unrequired `not.toBeNull()`

These types of checks should be asserted on another test suite rather.
  • Loading branch information
bsastregx authored Jan 7, 2025
1 parent c6ee077 commit a016a74
Show file tree
Hide file tree
Showing 13 changed files with 214 additions and 71 deletions.
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

0 comments on commit a016a74

Please sign in to comment.