Skip to content

Commit

Permalink
feat(popover-core): prevent internal logic (#839)
Browse files Browse the repository at this point in the history 
* fix(popover-core): prevent internal logic

* fix: additional rule

* feat: controlled vs uncontrolled

* fix: open must be out due to dependencies

* fix: open monitored in lifecycle

* docs: readme update

* Update packages/core/src/components/popover-core/popover-core.tsx

Co-authored-by: mJarsater <62651103+mJarsater@users.noreply.github.com>

* Update packages/core/src/components/popover-core/readme.md

Co-authored-by: mJarsater <62651103+mJarsater@users.noreply.github.com>

* chore: swap controlled and uncontrolled

* docs: update tooltip

* fix: rename defaultOpen to defaultShow

* chore: comment update

---------

Co-authored-by: mJarsater <62651103+mJarsater@users.noreply.github.com>
  • Loading branch information
@theJohnnyMe @mJarsater
theJohnnyMe and mJarsater authored Nov 14, 2024
1 parent d90f65d commit 819cbe5
Show file tree
Hide file tree
Showing 9 changed files with 192 additions and 50 deletions.
9 changes: 7 additions & 2 deletions packages/core/src/components/popover-canvas/popover-canvas.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,17 @@ export class TdsPopoverCanvas {
@Element() host: HTMLTdsPopoverCanvasElement;

/** The CSS-selector for an element that will trigger the Popover */
@Prop() selector: string = '';
@Prop() selector: string;

/** Element that will trigger the Popover (takes priority over selector) */
@Prop() referenceEl?: HTMLElement | null;

/** Decides if the component should be visible from the start. */
@Prop() defaultShow: boolean = false;

/** Controls whether the Popover is shown or not. If this is set hiding and showing
* will be decided by this prop and will need to be controlled from the outside.
* will be decided by this prop and will need to be controlled from the outside. This
* also means that clicking outside of the popover won't close it. Takes precedence over `defaultShow` prop.
*/
@Prop() show: boolean = null;

Expand Down Expand Up @@ -71,6 +75,7 @@ export class TdsPopoverCanvas {
ref={(el) => {
this.childRef = el;
}}
defaultShow={this.defaultShow}
>
<div>
{/* (@stencil/core@3.3.0): This div is somehow needed to keep the slotted children in a predictable order */}
Expand Down
49 changes: 40 additions & 9 deletions packages/core/src/components/popover-canvas/readme.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,20 +27,51 @@ use the `referenceEl` prop rather than the `selector` the referenced element can
</TdsPopoverCanvas>

```
<br>
### Controlled vs Uncontrolled

#### Uncontrolled mode
The popover component will be shown or hidden based on a `trigger` prop and `selector` or `referenceEl` prop.

Example:

```html
<button id="trigger">Open Popover</button>
<tds-popover-canvas [selector]="#trigger">
<h2 class="tds-headline-02 tds-u-mt0">A Popover Canvas!</h2>
</tds-popover-canvas>
```

#### Controlled mode
The `open` prop can be used to control the visibility of the popover, meaning that the popover will be shown or hidden based on the `show` prop. Props `selector` or `referenceEl` are still needed in order to determine the position of the popover.

Example:

```html
<button id="trigger" onClick="{() => showPopover = !showPopover}">Open Popover</button>
<tds-popover-canvas selector="#trigger" show="showPopover">
<h2 class="tds-headline-02 tds-u-mt0">A Popover Canvas!</h2>
</tds-popover-canvas>

```

**Note:** Examples above is just for demonstration purposes, update it according to your framework syntax.

<!-- Auto Generated Below -->


## Properties

| Property | Attribute | Description | Type | Default |
| ---------------- | ----------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | ----------- |
| `modifiers` | -- | Array of modifier objects to pass to popper.js. See https://popper.js.org/docs/v2/modifiers/ | `Object[]` | `[]` |
| `offsetDistance` | `offset-distance` | Sets the offset distance | `number` | `8` |
| `offsetSkidding` | `offset-skidding` | Sets the offset skidding | `number` | `0` |
| `placement` | `placement` | Decides the placement of the Popover Canvas. See https://popper.js.org/docs/v2/constructors/#placement | `"auto" \| "auto-end" \| "auto-start" \| "bottom" \| "bottom-end" \| "bottom-start" \| "left" \| "left-end" \| "left-start" \| "right" \| "right-end" \| "right-start" \| "top" \| "top-end" \| "top-start"` | `'auto'` |
| `referenceEl` | -- | Element that will trigger the Popover (takes priority over selector) | `HTMLElement` | `undefined` |
| `selector` | `selector` | The CSS-selector for an element that will trigger the Popover | `string` | `''` |
| `show` | `show` | Controls whether the Popover is shown or not. If this is set hiding and showing will be decided by this prop and will need to be controlled from the outside. | `boolean` | `null` |
| Property | Attribute | Description | Type | Default |
| ---------------- | ----------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | ----------- |
| `defaultShow` | `default-show` | Decides if the component should be visible from the start. | `boolean` | `false` |
| `modifiers` | -- | Array of modifier objects to pass to popper.js. See https://popper.js.org/docs/v2/modifiers/ | `Object[]` | `[]` |
| `offsetDistance` | `offset-distance` | Sets the offset distance | `number` | `8` |
| `offsetSkidding` | `offset-skidding` | Sets the offset skidding | `number` | `0` |
| `placement` | `placement` | Decides the placement of the Popover Canvas. See https://popper.js.org/docs/v2/constructors/#placement | `"auto" \| "auto-end" \| "auto-start" \| "bottom" \| "bottom-end" \| "bottom-start" \| "left" \| "left-end" \| "left-start" \| "right" \| "right-end" \| "right-start" \| "top" \| "top-end" \| "top-start"` | `'auto'` |
| `referenceEl` | -- | Element that will trigger the Popover (takes priority over selector) | `HTMLElement` | `undefined` |
| `selector` | `selector` | The CSS-selector for an element that will trigger the Popover | `string` | `undefined` |
| `show` | `show` | Controls whether the Popover is shown or not. If this is set hiding and showing will be decided by this prop and will need to be controlled from the outside. This also means that clicking outside of the popover won't close it. Takes precedence over `defaultShow` prop. | `boolean` | `null` |


## Methods
Expand Down
42 changes: 31 additions & 11 deletions packages/core/src/components/popover-core/popover-core.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,18 @@ export class TdsPopoverCore {
@Element() host!: HTMLTdsPopoverCoreElement;

/** The CSS-selector for an element that will trigger the pop-over */
@Prop() selector: string = '';
@Prop() selector: string;

/** Element that will trigger the pop-over (takes priority over selector) */
@Prop() referenceEl?: HTMLElement | null;

/** Decides if the Popover Menu should be visible from the start */
/** Decides if the component should be visible from the start. */
@Prop() defaultShow: boolean = false;

/** Controls whether the Popover is shown or not. If this is set hiding and showing
* will be decided by this prop and will need to be controlled from the outside. This
* also means that clicking outside of the popover won't close it. Takes precedence over `defaultShow` prop.
*/
@Prop() show: boolean = null;

/** Decides the placement of the Popover Menu */
Expand Down Expand Up @@ -59,6 +65,8 @@ export class TdsPopoverCore {

@State() isShown: boolean = false;

@State() disableLogic: boolean = false;

/** Property for closing popover programmatically */
@Method() async close() {
this.setIsShown(false);
Expand Down Expand Up @@ -105,11 +113,6 @@ export class TdsPopoverCore {
}
}

/* To enable initial loading of a component if user controls show prop*/
componentWillLoad() {
this.setIsShown(this.show);
}

/* To observe any change of show prop after an initial load */
@Watch('show')
onShowChange(newValue: boolean) {
Expand All @@ -118,7 +121,9 @@ export class TdsPopoverCore {

@Watch('referenceEl')
onReferenceElChanged(newValue: HTMLElement, oldValue: HTMLElement) {
if (newValue !== oldValue) this.initialize({ referenceEl: newValue, trigger: this.trigger });
if (newValue !== oldValue) {
this.initialize({ referenceEl: newValue, trigger: this.trigger });
}
}

@Watch('trigger')
Expand Down Expand Up @@ -152,7 +157,7 @@ export class TdsPopoverCore {
this.setIsShown(true);
}.bind(this);

private handleHide = function handleShow(event) {
private handleHide = function handleHide(event) {
event.stopPropagation();
this.setIsShown(false);
}.bind(this);
Expand Down Expand Up @@ -225,15 +230,30 @@ export class TdsPopoverCore {
this.popperInstance?.destroy();
}

componentDidLoad() {
connectedCallback() {
if (this.selector === undefined && this.referenceEl === undefined) {
this.disableLogic = true;
console.warn(
'TDS-POPOVER-CORE: Popover internal logic disabled. Please provide a `selector` or `referenceEl` prop',
);
return;
}

this.initialize({
referenceEl: this.referenceEl,
trigger: this.trigger,
});
}

/* To enable initial loading of a component if user controls show prop*/
componentWillLoad() {
if (this.show === true || this.defaultShow === true) {
this.setIsShown(true);
}
}

componentDidRender() {
if (this.isShown && !this.renderedShowValue) {
if (this.isShown && !this.renderedShowValue && !this.disableLogic) {
// Here we update the popper position since its position is wrong
// before it is rendered.
this.popperInstance.update();
Expand Down
Loading

0 comments on commit 819cbe5

Please sign in to comment.