Skip to content

Commit

Permalink
fix(parent-visibility): Make changes for v2 (#11797)
Browse files Browse the repository at this point in the history
### Related Ticket(s)

None

### Description

We created the ParentVisibility mixin in v1 to address problematic interactions between components that used the `sameHeight` utility and components that conditionally show/hide child elements. It was designed to work on any element that potentially hid content, but in practice Tabs was the only one that we ever experienced problems with.

The Tabs component underwent fundamental changes in v2, and as a result, the ParentVisibility mixin no longer works. Instead of refactoring it, we can use new CSS specs to replace the sameHeight behavior while removing a bunch of JavaScript.

While working on this, I noticed a visual regression. The CTA Block content items' CTAs stack in v1, but that wasn't happening in v2. A simple bit of CSS fixes this.

**v1:**
<img width="391" alt="Screenshot 2024-05-20 at 9 28 01 AM" src="https://github.com/carbon-design-system/carbon-for-ibm-dotcom/assets/12532727/cb91d25d-1f9e-42d2-9e5e-a8cb73e25934">

**v2:**
<img width="391" alt="Screenshot 2024-05-20 at 9 28 10 AM" src="https://github.com/carbon-design-system/carbon-for-ibm-dotcom/assets/12532727/0a85087d-e4f0-4498-a8f7-cf6878cdb243">

### Changelog

**Changed**

- Deprecates `ParentVisibility` mixin.
- Replaces `<c4d-cta-block-item-row>`'s sameHeight JavaScript logic with CSS subgrid styles.
- Fixes `<c4d-cta-block>`'s slotted CTA items visual regression from v1 to v2.
- Fixes "CTA Block > Within Tabs" story. 

### Testing

Visit [the CTA Block > With content items deploy preview story](https://ibmdotcom-webcomponents.s3.us-east.cloud-object-storage.appdomain.cloud/deploy-previews/11797/index.html?path=/story/components-cta-block--with-content-items) and verify the content items each have the same height as each other, and that each of their sub-elements (e.g. heading, description text, etc.) have the same heights.

Verify the same for content items in each tab in [the CTA Block > Within Tabs deploy preview story](https://ibmdotcom-webcomponents.s3.us-east.cloud-object-storage.appdomain.cloud/deploy-previews/11797/index.html?path=/story/components-cta-block--within-tabs).

<!-- React and Web Component deploy previews are enabled by default. -->
<!-- To enable additional available deploy previews, apply the following -->
<!-- labels for the corresponding package: -->
<!-- *** "test: e2e": Codesandbox examples and e2e integration tests -->
<!-- *** "package: services": Services -->
<!-- *** "package: utilities": Utilities -->
<!-- *** "RTL": React / Web Components (RTL) -->
<!-- *** "feature flag": React / Web Components (experimental) -->
  • Loading branch information
jkaeser authored May 21, 2024
1 parent a012282 commit b6911a8
Show file tree
Hide file tree
Showing 5 changed files with 30 additions and 155 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ const { stablePrefix: c4dPrefix } = settings;
* @param Base The base class.
* @returns A mix-in implementing the logic for handling elements that need to
* respond when a potential parent element becomes visible.
* @deprecated This mixin will be removed in a future release.
*/
const ParentVisibilityMixin = <T extends Constructor<HTMLElement>>(Base: T) => {
abstract class ParentVisibilityMixinImpl extends Base {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import textNullable from '../../../../.storybook/knob-text-nullable';
import '../../tabs-extended/index';
import '../index';
import '../../link-list/index';
import { prefix } from '../../../internal/vendor/@carbon/web-components/globals/settings';

import content from '../../cta-section/__stories__/content';

Expand Down Expand Up @@ -219,26 +220,37 @@ export const WithinTabs = (args) => {
const { contentItemType, contentItemCount } = args?.WithinTabs ?? {};

return html`
<c4d-tabs-extended orientation="horizontal">
<c4d-tab label="Tab 1" selected>
<c4d-tabs-extended orientation="horizontal" value="first">
<c4d-tab id="tab-first" target="panel-first" value="first">Tab 1</c4d-tab>
<c4d-tab id="tab-second" target="panel-second" value="second"
>Tab 2</c4d-tab
>
<c4d-tab id="tab-third" target="panel-third" value="third">Tab 3</c4d-tab>
</c4d-tabs-extended>
<div class="${prefix}-ce-demo-devenv--tab-panels">
<div id="panel-first" role="tabpanel" aria-labelledby="tab-first" hidden>
<c4d-cta-block>
<c4d-content-block-heading>Tab 1</c4d-content-block-heading>
${renderItems(contentItemType, contentItemCount)}
</c4d-cta-block>
</c4d-tab>
<c4d-tab label="Tab 2">
</div>
<div
id="panel-second"
role="tabpanel"
aria-labelledby="tab-second"
hidden>
<c4d-cta-block>
<c4d-content-block-heading>Tab 2</c4d-content-block-heading>
${renderItems(contentItemType, contentItemCount)}
</c4d-cta-block>
</c4d-tab>
<c4d-tab label="Tab 3">
</div>
<div id="panel-third" role="tabpanel" aria-labelledby="tab-third" hidden>
<c4d-cta-block>
<c4d-content-block-heading>Tab 3</c4d-content-block-heading>
${renderItems(contentItemType, contentItemCount)}
</c4d-cta-block>
</c4d-tab>
</c4d-tabs-extended>
</div>
</div>
`;
};

Expand Down
140 changes: 3 additions & 137 deletions packages/web-components/src/components/cta-block/cta-block-item-row.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,9 @@
*/

import { LitElement, html } from 'lit';
import { property, state } from 'lit/decorators.js';
import { property } from 'lit/decorators.js';
import settings from '../../internal/vendor/@carbon/ibmdotcom-utilities/utilities/settings/settings';
import sameHeight from '../../internal/vendor/@carbon/ibmdotcom-utilities/utilities/sameHeight/sameHeight';
import StableSelectorMixin from '../../globals/mixins/stable-selector';
import ParentVisibilityMixin from '../../component-mixins/parent-visibility/parent-visibility';

import styles from './cta-block.scss';
import { carbonElement as customElement } from '../../internal/vendor/@carbon/web-components/globals/decorators/carbon-element';

Expand All @@ -26,123 +23,13 @@ const { prefix, stablePrefix: c4dPrefix } = settings;
* @slot .
*/
@customElement(`${c4dPrefix}-cta-block-item-row`)
class C4DCTABlockItemRow extends ParentVisibilityMixin(
StableSelectorMixin(LitElement)
) {
class C4DCTABlockItemRow extends StableSelectorMixin(LitElement) {
/** Defines if the bottom border is rendered */
@property({ type: Boolean, reflect: true, attribute: 'no-border' })
_noBorder = false;

/**
* Array to hold the card-heading elements within child items.
*/
private _childItemHeadings: any[] = [];

/**
* Array to hold the text copy elements within child items.
*/
private _childItemCopies: any[] = [];

/**
* The observer for the resize of the viewport.
*/
private _observerResizeRoot: any | null = null; // TODO: Wait for `.d.ts` update to support `ResizeObserver`

public _onParentVisible() {
this._setSameHeight();
}

/**
* Cleans-up and creats the resize observer for the scrolling container.
*
* @param [options] The options.
* @param [options.create] `true` to create the new resize observer.
*/
private _cleanAndCreateObserverResize({ create }: { create?: boolean } = {}) {
if (this._observerResizeRoot) {
this._observerResizeRoot.disconnect();
this._observerResizeRoot = null;
}
if (create) {
// TODO: Wait for `.d.ts` update to support `ResizeObserver`
// @ts-ignore
this._observerResizeRoot = new ResizeObserver(this._setSameHeight);
this._observerResizeRoot.observe(this.ownerDocument!.documentElement);
}
}

/**
* The observer for the resize of the viewport, calls sameHeight utility function
*/
private _setSameHeight = () => {
window.requestAnimationFrame(() => {
sameHeight(
this._childItemHeadings.filter((e) => {
return e;
}),
'md'
);

sameHeight(
this._childItemCopies.filter((e) => {
return e;
}),
'md'
);
});
};

/**
* `true` if there are CTA action in the content item area.
*/
@state()
protected _hasAction = false;

/**
* Handles `slotchange` event, also sets height to all headings to the tallest one.
*
* @param event The event.
*/
protected _handleSlotChange(event: Event) {
const { target } = event;
const { selectorItem, selectorItemHeading, selectorItemCopy } = this
.constructor as typeof C4DCTABlockItemRow;

const childItems = (target as HTMLSlotElement)
.assignedNodes()
.filter((elem) => (elem as HTMLElement).matches?.(selectorItem));

if (childItems) {
childItems.forEach((e) => {
this._childItemHeadings.push(
(e as HTMLElement).querySelector(selectorItemHeading)
);
this._childItemCopies.push(
(e as HTMLElement).querySelector(selectorItemCopy)
);
});
}

this._setSameHeight();
}

render() {
return html` <slot @slotchange="${this._handleSlotChange}"></slot> `;
}

connectedCallback() {
super.connectedCallback();
this._cleanAndCreateObserverResize({ create: true });
}

disconnectedCallback() {
this._cleanAndCreateObserverResize();
super.disconnectedCallback();
}

firstUpdated() {
super.connectedCallback();
this._cleanAndCreateObserverResize({ create: true });
return html`<slot></slot>`;
}

/**
Expand All @@ -161,27 +48,6 @@ class C4DCTABlockItemRow extends ParentVisibilityMixin(
return `${c4dPrefix}--cta-block-item-row`;
}

/**
* A selector that will return the CTA Section item
*/
static get selectorItem() {
return `${c4dPrefix}-cta-block-item`;
}

/**
* A selector that will return the CTA Section item's heading
*/
static get selectorItemHeading() {
return `${c4dPrefix}-content-item-heading`;
}

/**
* A selector that will return the CTA Section item's copy
*/
static get selectorItemCopy() {
return `${c4dPrefix}-content-item-copy`;
}

static styles = styles; // `styles` here is a `CSSResult` generated by custom WebPack loader
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,8 +167,9 @@
@include make-col-ready;

position: relative;
display: flex;
flex-direction: column;
display: grid;
grid-row: auto / span 5;
grid-template-rows: subgrid;
inline-size: 100%;
margin-block: 0;
margin-block-end: 0;
Expand Down Expand Up @@ -202,6 +203,8 @@
}

.#{$prefix}--content-item__cta {
display: flex;
flex-direction: column;
margin-block: 0;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import '../image/image';
import styles from './video-player.scss';
import StableSelectorMixin from '../../globals/mixins/stable-selector';
import C4DVideoPlayerContainer from './video-player-container';
import ParentVisibilityMixin from '../../component-mixins/parent-visibility/parent-visibility';
import { carbonElement as customElement } from '../../internal/vendor/@carbon/web-components/globals/decorators/carbon-element.js';

export { VIDEO_PLAYER_CONTENT_STATE };
Expand All @@ -38,9 +37,7 @@ const { stablePrefix: c4dPrefix } = settings;
* @element c4d-video-player
*/
@customElement(`${c4dPrefix}-video-player`)
class C4DVideoPlayer extends FocusMixin(
StableSelectorMixin(ParentVisibilityMixin(LitElement))
) {
class C4DVideoPlayer extends FocusMixin(StableSelectorMixin(LitElement)) {
/**
* The video player's mode showing Inline or Lightbox.
*/
Expand Down Expand Up @@ -118,10 +115,6 @@ class C4DVideoPlayer extends FocusMixin(
}
}

public _onParentVisible() {
this._updateThumbnailUrl();
}

/**
* userInitiatedTogglePlaybackState
*/
Expand Down

0 comments on commit b6911a8

Please sign in to comment.