Skip to content

Commit

Permalink
feat(locale-modal): only render modal when its called by user click (#…
Browse files Browse the repository at this point in the history
…10892)

### Related Ticket(s)

Closes #10657 

### Description

The locale modal is being rendered upon page load, though it is not visible and users cannot interact with it. This unnecessarily increases the DOM node count. This PR updates the functionality so that the modal itself is not rendered until it is called by a user clicking the locale switcher button in the footer. 

### Analysis
#### Default Footer Story's `<dds-locale-modal-composite>`
<!-- | |Nodes|Comments|Inters|
|---|---|---|---|
|Before|4197|1145|1723|
|After|1458|373|566|
|Reduction|65%|67%|63%| -->

| |Nodes|Comments|Inters|
|---|---|---|---|
|Before|2734|770|1155|
|After (no region selected)|310 (-89%)|62 (-92%)|146 (-87%)|
|After (Americas region selected)|1163 (-57%)|309 (-60%)|503 (-56%)|

### Testing
In the deploy preview, visit the default Footer story. Verify the locale modal hasn't loaded into the DOM until you click the locale button in the footer.
1. Find the `<div style="display: contents;">` using the browser dev tools. This is the _modal render root_, and it should be empty.
2. Click the locale button in the footer. The locale modal should load into the modal render root and appear in the viewport with a fade-in animation.
3. Close the modal. The modal should close with a fade-out animation and the DOM content should be removed.

Next, verify the selectable regions' locale options do not load into the DOM until a region is selected.
1. With your browser tools still open, re-open the locale modal. Note that there aren't any elements slotted into the `<dds-locale-search>`.
2. Select a region. The relevant `<dds-locale-items>` should load into the DOM and appear within the modal. You can confirm the correct items appear by verifying their "region" attribute matches the selected region.
3. Go back to the region selector. The locale items should get pruned from the DOM.

It's worth repeating the above steps in the Dotcom Shell default Story, too.

### Changelog

**Changed**

- Locale modal no longer renders in the DOM unless the locale button in the footer is clicked by user; locales do not render until user selects a region.
- Adds support in ModalRenderMixin for rendering modal only when specified properties update. This is optional, and the original behavior is preserved as the default.

<!-- 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
pjudge committed Oct 19, 2023
1 parent baa7af3 commit 317d2be
Show file tree
Hide file tree
Showing 10 changed files with 182 additions and 116 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -286,12 +286,6 @@ class DDSDotcomShellComposite extends LitElement {
@property({ attribute: 'selected-menu-item' })
selectedMenuItem!: string;

/**
* `true` to open the locale modal. This goes to footer.
*/
@property({ type: Boolean, attribute: 'open-locale-modal' })
openLocaleModal = false;

/**
* Footer size. This goes to footer.
*/
Expand Down Expand Up @@ -385,7 +379,6 @@ class DDSDotcomShellComposite extends LitElement {
localeList,
footerLinks,
footerSize,
openLocaleModal,
openSearchDropdown,
navLinks,
hasProfile,
Expand Down Expand Up @@ -452,7 +445,6 @@ class DDSDotcomShellComposite extends LitElement {
legalLinks,
links: footerLinks,
localeList,
openLocaleModal,
selectedLanguage,
size: footerSize,
_loadLocaleList,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -484,10 +484,10 @@ class DDSExpressiveModal extends StableSelectorMixin(
this.ownerDocument.body.style.overflow = 'hidden';
this.removeAttribute('aria-hidden');
this._launcher = this.ownerDocument!.activeElement;
await this._waitForTransitionEnd();
const primaryFocusNode = this.querySelector(
(this.constructor as typeof DDSExpressiveModal).selectorPrimaryFocus
);
await this._waitForTransitionEnd();
if (primaryFocusNode) {
// For cases where a `carbon-web-components` component (e.g. `<bx-btn>`) being `primaryFocusNode`,
// where its first update/render cycle that makes it focusable happens after `<bx-modal>`'s first update/render cycle
Expand Down
47 changes: 33 additions & 14 deletions packages/web-components/src/components/footer/footer-composite.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,13 @@ import './language-selector-mobile';
import '../../internal/vendor/@carbon/web-components/components/combo-box/combo-box-item.js';
import '../../internal/vendor/@carbon/web-components/components/select/select-item.js';
import { carbonElement as customElement } from '../../internal/vendor/@carbon/web-components/globals/decorators/carbon-element.js';
import { moderate02 } from '@carbon/motion';

const { stablePrefix: ddsPrefix } = ddsSettings;

// Delay matches the CSS animation timing for fadein/out of modal.
const delay = parseInt(moderate02, 10);

/**
* Component that rendres footer from inks data.
*
Expand All @@ -61,9 +65,15 @@ class DDSFooterComposite extends MediaQueryMixin(
/**
* Handles `click` event on the locale button.
*/
private _handleClickLocaleButton = () => {
private async _handleClickLocaleButton() {
this.openLocaleModal = true;
};
// Set 'open' attribute after modal is in dom so CSS can fade it in.
await this.updateComplete;
const composite = this.modalRenderRoot?.querySelector(
'dds-locale-modal-composite'
);
composite?.setAttribute('open', '');
}

@state()
_isMobile = this.carbonBreakpoints.lg.matches;
Expand All @@ -79,7 +89,10 @@ class DDSFooterComposite extends MediaQueryMixin(
// @ts-ignore: The decorator refers to this method but TS thinks this method is not referred to
private _handleCloseModal = (event: CustomEvent) => {
if ((this.modalRenderRoot as Element).contains(event.target as Node)) {
this.openLocaleModal = false;
// Timeout here ensures the modal closing animation is visible.
setTimeout(() => {
this.openLocaleModal = false;
}, delay);
}
};

Expand Down Expand Up @@ -194,7 +207,12 @@ class DDSFooterComposite extends MediaQueryMixin(
* `true` to open the locale modal.
*/
@property({ type: Boolean, attribute: 'open-locale-modal' })
openLocaleModal = false;
openLocaleModal;

/**
* @inheritdoc
*/
modalTriggerProps = ['openLocaleModal', 'localeList'];

/**
* Footer size.
Expand Down Expand Up @@ -245,16 +263,17 @@ class DDSFooterComposite extends MediaQueryMixin(
openLocaleModal,
_loadLocaleList: loadLocaleList,
} = this;
return html`
<dds-locale-modal-composite
lang-display="${ifNonNull(langDisplay)}"
language="${ifNonNull(language)}"
?open="${openLocaleModal}"
.collatorCountryName="${ifNonNull(collatorCountryName)}"
.localeList="${ifNonNull(localeList)}"
._loadLocaleList="${ifNonNull(loadLocaleList)}">
</dds-locale-modal-composite>
`;
return openLocaleModal
? html`
<dds-locale-modal-composite
lang-display="${ifNonNull(langDisplay)}"
language="${ifNonNull(language)}"
.collatorCountryName="${ifNonNull(collatorCountryName)}"
.localeList="${ifNonNull(localeList)}"
._loadLocaleList="${ifNonNull(loadLocaleList)}">
</dds-locale-modal-composite>
`
: html``;
}

renderLanguageSelector(slot = 'language-selector') {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,15 @@ import ifNonNull from '../../internal/vendor/@carbon/web-components/globals/dire
import LocaleAPI from '@carbon/ibmdotcom-services/es/services/Locale/Locale.js';
import ddsSettings from '../../internal/vendor/@carbon/ibmdotcom-utilities/utilities/settings/settings';
import altlangs from '../../internal/vendor/@carbon/ibmdotcom-utilities/utilities/altlangs/altlangs.js';
import HostListener from '../../internal/vendor/@carbon/web-components/globals/decorators/host-listener.js';
import HostListenerMixin from '../../internal/vendor/@carbon/web-components/globals/mixins/host-listener.js';
import HybridRenderMixin from '../../globals/mixins/hybrid-render';
import {
Country,
LocaleList,
} from '../../internal/vendor/@carbon/ibmdotcom-services-store/types/localeAPI.d';
import './locale-modal';
import DDSLocaleModal from './locale-modal';
import './regions';
import './region-item';
import './locale-search';
Expand All @@ -33,7 +36,9 @@ const { stablePrefix: ddsPrefix } = ddsSettings;
* @element dds-locale-modal-composite
*/
@customElement(`${ddsPrefix}-locale-modal-composite`)
class DDSLocaleModalComposite extends HybridRenderMixin(LitElement) {
class DDSLocaleModalComposite extends HostListenerMixin(
HybridRenderMixin(LitElement)
) {
/**
* @param countries A country list.
* @returns Sorted version of the given country list.
Expand Down Expand Up @@ -88,6 +93,17 @@ class DDSLocaleModalComposite extends HybridRenderMixin(LitElement) {
@property({ type: Boolean })
open = false;

/**
* The region chosen by user.
*/
@property()
chosenRegion?: string;

@HostListener(DDSLocaleModal.eventRegionUpdated)
protected _handleRegionUpdatedEvent(event) {
this.chosenRegion = event.detail.region || undefined;
}

// eslint-disable-next-line class-methods-use-this
async getLangDisplay() {
const response = await LocaleAPI.getLangDisplay();
Expand Down Expand Up @@ -117,7 +133,7 @@ class DDSLocaleModalComposite extends HybridRenderMixin(LitElement) {
}

renderLightDOM() {
const { langDisplay, localeList, open } = this;
const { chosenRegion, langDisplay, localeList, open } = this;
const { localeModal, regionList } = localeList ?? {};
const {
availabilityText,
Expand Down Expand Up @@ -167,7 +183,6 @@ class DDSLocaleModalComposite extends HybridRenderMixin(LitElement) {
language: string;
}[]
);

return html`
<dds-locale-modal
close-button-assistive-text="${ifNonNull(modalClose)}"
Expand All @@ -176,32 +191,43 @@ class DDSLocaleModalComposite extends HybridRenderMixin(LitElement) {
?open="${open}">
<dds-regions title="${ifNonNull(headerTitle)}">
${regionList?.map(({ countryList, name }) => {
const isInvalid =
countryList.length === 0 ||
massagedCountryList?.find(({ region }) => region === name) ===
undefined;
return html`
<dds-region-item
?invalid="${countryList.length === 0 ||
massagedCountryList?.find(({ region }) => region === name) ===
undefined}"
?invalid="${isInvalid}"
name="${name}"></dds-region-item>
`;
})}
</dds-regions>
<dds-locale-search
close-button-assistive-text="${ifNonNull(searchClearText)}"
label-text="${ifNonNull(searchLabel)}"
placeholder="${ifNonNull(searchPlaceholder)}"
availability-label-text="${ifNonNull(availabilityText)}"
unavailability-label-text="${ifNonNull(unavailabilityText)}">
${massagedCountryList?.map(
({ country, href, language, locale, region }) => html`
<dds-locale-item
country="${country}"
href="${href}"
language="${language}"
locale="${locale}"
region="${region}">
</dds-locale-item>
`
)}
${chosenRegion
? html`
${massagedCountryList
?.filter(({ region }) => {
return region === chosenRegion;
})
.map(
({ country, href, language, locale, region }) => html`
<dds-locale-item
country="${country}"
href="${href}"
language="${language}"
locale="${locale}"
region="${region}">
</dds-locale-item>
`
)}
`
: ``}
</dds-locale-search>
</dds-locale-modal>
`;
Expand Down
73 changes: 55 additions & 18 deletions packages/web-components/src/components/locale-modal/locale-modal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,8 @@ class DDSLocaleModal extends DDSExpressiveModal {
* Handles `click` event on the back button.
*/
private _handleClickBackButton(e) {
this._currentRegion = undefined;
e.preventDefault();
this._currentRegion = undefined;
}

/**
Expand All @@ -71,6 +71,31 @@ class DDSLocaleModal extends DDSExpressiveModal {
this._currentRegion = undefined;
}

/**
* Sets focus on primary selectable element.
*/
private async _setPrimaryFocus() {
const { selectorPrimaryFocus } = DDSLocaleModal;
const focusTarget = this.querySelector(selectorPrimaryFocus);
if (focusTarget) {
(focusTarget as HTMLElement).tabIndex = 0;
(focusTarget as HTMLElement).focus();
}
}

/**
* Sets focus on locale selector search.
*/
private async _setSearchFocus() {
const { selectorLocaleSearch } = DDSLocaleModal;
await this.updateComplete;
const localeSearch = this.querySelector(selectorLocaleSearch);
if (localeSearch) {
(localeSearch as DDSLocaleSearch).reset();
(localeSearch as HTMLElement).focus();
}
}

/**
* @returns The heading content for the region selector page.
*/
Expand Down Expand Up @@ -198,29 +223,38 @@ class DDSLocaleModal extends DDSExpressiveModal {
async updated(changedProperties) {
super.updated(changedProperties);
if (changedProperties.has('_currentRegion')) {
// Allow listening components to update their state.
this.dispatchEvent(
new CustomEvent(
(this.constructor as typeof DDSLocaleModal).eventRegionUpdated,
{
bubbles: true,
composed: true,
cancelable: true,
detail: {
region: this._currentRegion,
},
}
)
);

// Pass state to search element.
const { selectorLocaleSearch } = this
.constructor as typeof DDSLocaleModal;
const localeSearch = this.querySelector(selectorLocaleSearch);
if (localeSearch) {
(localeSearch as DDSLocaleSearch).region = this._currentRegion ?? '';

if (this.open) {
(localeSearch as DDSLocaleSearch).reset();
(localeSearch as HTMLElement).focus();
}
}

// re-focus on first region-item when navigating back to the first modal pane
const { selectorPrimaryFocus } = this
.constructor as typeof DDSLocaleModal;
const activeRegion = this.querySelector(selectorPrimaryFocus);
if (activeRegion && this.open) {
(activeRegion as HTMLElement).tabIndex = 0;
(activeRegion as HTMLElement).focus();
}
// Set element focus.
this._currentRegion ? this._setSearchFocus() : this._setPrimaryFocus();
}
}

static get stableSelector() {
return `${ddsPrefix}--locale-modal`;
}

/**
* A selector selecting the locale search UI.
*/
Expand All @@ -238,10 +272,6 @@ class DDSLocaleModal extends DDSExpressiveModal {
`;
}

static get stableSelector() {
return `${ddsPrefix}--locale-modal`;
}

/**
* A selector selecting tabbable nodes.
*/
Expand All @@ -256,6 +286,13 @@ class DDSLocaleModal extends DDSExpressiveModal {
`;
}

/**
* Name for event fired when a region is selected.
*/
static get eventRegionUpdated() {
return `${ddsPrefix}-locale-modal-region-updated`;
}

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 @@ -75,19 +75,17 @@ class DDSLocaleSearch extends ThrottedInputMixin(
private _updateSearchResults(searchText: string) {
const { selectorItem } = this.constructor as typeof DDSLocaleSearch;
const { region: currentRegion, _liveRegion: liveRegion } = this;
let hasMatch = false;
let count = 0;
forEach(this.querySelectorAll(selectorItem), (item) => {
const { country, language, region } = item as DDSLocaleItem;
const matches =
region === currentRegion && search([country, language], searchText);
if (matches) {
hasMatch = true;
count++;
}
(item as HTMLElement).hidden = !matches;
});
this._hasAvailableItem = hasMatch;
this._hasAvailableItem = count > 0;
if (liveRegion) {
const announcement = count === 1 ? `${count} result` : `${count} results`;
liveRegion.innerText = announcement;
Expand Down
Loading

0 comments on commit 317d2be

Please sign in to comment.