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

fix: usage of @tracked modifier across components #2488

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
13 changes: 13 additions & 0 deletions .changeset/purple-poets-smoke.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
---
"@hashicorp/design-system-components": patch
---

Fixed usage of the `@tracked` modifier in several components:
* `MaskedInput`
* `TextInput`
* `Pagination::Compact`
* `Pagination::Numbered`
* `SideNav`
* `Table`
* `Table::ThSelectable`
* `Tabs`
32 changes: 7 additions & 25 deletions packages/components/src/components/hds/form/masked-input/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@
*/

import Component from '@glimmer/component';
import { tracked } from '@glimmer/tracking';
import { action } from '@ember/object';
import { getElementId } from '../../../../utils/hds-get-element-id.ts';
import type { HdsCopyButtonSignature } from '../../copy/button/index.ts';
import type { HdsFormVisibilityToggleSignature } from '../visibility-toggle/index.ts';
import { tracked } from '@glimmer/tracking';

export interface HdsFormMaskedInputBaseSignature {
Args: {
Expand All @@ -28,25 +28,22 @@ export interface HdsFormMaskedInputBaseSignature {
}

export default class HdsFormMaskedInputBase extends Component<HdsFormMaskedInputBaseSignature> {
@tracked isContentMasked = this.args.isContentMasked ?? true;
@tracked isContentMasked: boolean;

constructor(owner: unknown, args: HdsFormMaskedInputBaseSignature['Args']) {
super(owner, args);
this.isContentMasked = this.args.isContentMasked ?? true;
}

@action
onClickToggleMasking(): void {
this.isContentMasked = !this.isContentMasked;
}

/**
* Calculates the unique ID to assign to the form control
*/
get id(): string {
return getElementId(this);
}

/**
* @param ariaLabel
* @type {string}
* @default 'Show masked content'
*/
get visibilityToggleAriaLabel(): string {
if (this.args.visibilityToggleAriaLabel) {
return this.args.visibilityToggleAriaLabel;
Expand All @@ -57,11 +54,6 @@ export default class HdsFormMaskedInputBase extends Component<HdsFormMaskedInput
}
}

/**
* @param ariaMessageText
* @type {string}
* @default 'Input content is now hidden'
*/
get visibilityToggleAriaMessageText(): string {
if (this.args.visibilityToggleAriaMessageText) {
return this.args.visibilityToggleAriaMessageText;
Expand All @@ -72,11 +64,6 @@ export default class HdsFormMaskedInputBase extends Component<HdsFormMaskedInput
}
}

/**
* @param copyButtonText
* @type {string}
* @default 'Copy masked content'
*/
get copyButtonText(): string {
if (this.args.copyButtonText) {
return this.args.copyButtonText;
Expand All @@ -85,11 +72,6 @@ export default class HdsFormMaskedInputBase extends Component<HdsFormMaskedInput
}
}

/**
* Get the class names to apply to the component.
* @method classNames
* @return {string} The "class" attribute to apply to the component.
*/
get classNames(): string {
const classes = ['hds-form-masked-input'];

Expand Down
27 changes: 10 additions & 17 deletions packages/components/src/components/hds/form/text-input/field.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,23 +40,21 @@ interface HdsFormTextInputFieldSignature {

export default class HdsFormTextInputField extends Component<HdsFormTextInputFieldSignature> {
@tracked isPasswordMasked = true;
@tracked hasVisibilityToggle = this.args.hasVisibilityToggle ?? true;
@tracked type = this.args.type ?? 'text';
@tracked type;

constructor(owner: unknown, args: HdsFormTextInputFieldSignature['Args']) {
super(owner, args);
this.type = this.args.type ?? 'text';
}

get hasVisibilityToggle(): boolean {
return this.args.hasVisibilityToggle ?? true;
}

/**
* @param showVisibilityToggle
* @type {boolean}
* @default false
*/
get showVisibilityToggle(): boolean {
return this.args.type === 'password' && this.hasVisibilityToggle;
}

/**
* @param visibilityToggleAriaLabel
* @type {string}
* @default 'Show password'
*/
get visibilityToggleAriaLabel(): string | undefined {
if (this.args.visibilityToggleAriaLabel) {
return this.args.visibilityToggleAriaLabel;
Expand All @@ -67,11 +65,6 @@ export default class HdsFormTextInputField extends Component<HdsFormTextInputFie
}
}

/**
* @param visibilityToggleAriaMessageText
* @type {string}
* @default 'Password is now hidden'
*/
get visibilityToggleAriaMessageText(): string | undefined {
if (this.args.visibilityToggleAriaMessageText) {
return this.args.visibilityToggleAriaMessageText;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ export default class HdsPaginationCompact extends Component<HdsPaginationCompact
// In the second case, the variable stores *only* the initial state of the component (coming from the arguments)
// at rendering time, but from that moment on it's not updated anymore, no matter what interaction the user
// has with the component (the state is controlled externally, eg. via query parameters)
@tracked _currentPageSize = this.args.currentPageSize ?? this.pageSizes[0];
@tracked _currentPageSize;
@tracked isControlled;

showLabels = this.args.showLabels ?? true; // if the labels for the "prev/next" controls are visible
Expand Down Expand Up @@ -123,6 +123,8 @@ export default class HdsPaginationCompact extends Component<HdsPaginationCompact
);
this.isControlled = true;
}

this._currentPageSize = this.args.currentPageSize ?? this.pageSizes[0];
}

get ariaLabel(): string {
Expand Down
30 changes: 18 additions & 12 deletions packages/components/src/components/hds/pagination/numbered/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -159,17 +159,8 @@ export const elliptize = ({
return result;
};
export default class HdsPaginationNumbered extends Component<HdsPaginationNumberedSignature> {
// These two private variables are used to differentiate between
// "uncontrolled" component (where the state is handled internally) and
// "controlled" component (where the state is handled externally, by the consumer's code).
// In the first case, these variables store the internal state of the component at any moment,
// and their value is updated internally according to the user's interaction with the component.
// In the second case, these variables store *only* the initial state of the component (coming from the arguments)
// at rendering time, but from that moment on they're not updated anymore, no matter what interaction the user
// has with the component (the state is controlled externally, eg. via query parameters)
@tracked _currentPage = this.args.currentPage ?? 1;
// we assert that `this.pageSizes` will always be an array with at least one item
@tracked _currentPageSize = this.args.currentPageSize ?? this.pageSizes[0]!;
@tracked _currentPage;
@tracked _currentPageSize;
@tracked isControlled;

showInfo = this.args.showInfo ?? true; // if the "info" block is visible
Expand Down Expand Up @@ -217,6 +208,18 @@ export default class HdsPaginationNumbered extends Component<HdsPaginationNumber
'@totalItems for "Hds::Pagination::Numbered" must be defined as an integer number',
typeof this.args.totalItems === 'number'
);

// These two private variables are used to differentiate between
// "uncontrolled" component (where the state is handled internally) and
// "controlled" component (where the state is handled externally, by the consumer's code).
// In the first case, these variables store the internal state of the component at any moment,
// and their value is updated internally according to the user's interaction with the component.
// In the second case, these variables store *only* the initial state of the component (coming from the arguments)
// at rendering time, but from that moment on they're not updated anymore, no matter what interaction the user
// has with the component (the state is controlled externally, eg. via query parameters)
this._currentPage = this.args.currentPage ?? 1;
// we assert that `this.pageSizes` will always be an array with at least one item
this._currentPageSize = this.args.currentPageSize ?? this.pageSizes[0];
}

get ariaLabel(): string {
Expand Down Expand Up @@ -248,7 +251,6 @@ export default class HdsPaginationNumbered extends Component<HdsPaginationNumber
if (this.isControlled) {
// noop
} else {
// if `this.isControlled` is `false`
this._currentPage = value as number;
}
}
Expand All @@ -258,6 +260,10 @@ export default class HdsPaginationNumbered extends Component<HdsPaginationNumber
// if the component is controlled, `@currentPageSize` is asserted to be a number
return this.args.currentPageSize as number;
} else {
if (this._currentPageSize === undefined) {
return 1;
}

return this._currentPageSize;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ export interface SetupPrimitivePopoverModifier {
}

export default class HdsPopoverPrimitive extends Component<HdsPopoverPrimitiveSignature> {
@tracked isOpen = this.args.isOpen ?? false;
@tracked isOpen;
@tracked isClosing = false;
containerElement?: HTMLElement;
toggleElement?: HTMLButtonElement;
Expand All @@ -70,6 +70,12 @@ export default class HdsPopoverPrimitive extends Component<HdsPopoverPrimitiveSi
enableClickEvents = this.args.enableClickEvents ?? false;
timer?: ReturnType<typeof setTimeout> | null;

constructor(owner: unknown, args: HdsPopoverPrimitiveSignature['Args']) {
super(owner, args);

this.isOpen = this.args.isOpen ?? false;
}

setupPrimitiveContainer = modifier<SetupPrimitiveContainerModifier>(
(element: HTMLElement): void => {
this.containerElement = element;
Expand Down
15 changes: 12 additions & 3 deletions packages/components/src/components/hds/side-nav/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,7 @@ interface HdsSideNavSignature {
}

export default class HdsSideNav extends Component<HdsSideNavSignature> {
@tracked isResponsive = this.args.isResponsive ?? true; // controls if the component reacts to viewport changes
@tracked isMinimized = this.args.isMinimized ?? false; // sets the default state on 'desktop' viewports
@tracked isCollapsible = this.args.isCollapsible ?? false; // controls if users can collapse the sidenav on 'desktop' viewports
@tracked isMinimized; // sets the default state on 'desktop' viewports
@tracked isAnimating = false;
@tracked isDesktop = true;
desktopMQ: MediaQueryList;
Expand All @@ -67,6 +65,7 @@ export default class HdsSideNav extends Component<HdsSideNavSignature> {

constructor(owner: unknown, args: HdsSideNavSignature['Args']) {
super(owner, args);
this.isMinimized = this.args.isMinimized ?? false;
this.desktopMQ = window.matchMedia(`(min-width:${this.desktopMQVal})`);
this.addEventListeners();
registerDestructor(this, (): void => {
Expand Down Expand Up @@ -97,6 +96,16 @@ export default class HdsSideNav extends Component<HdsSideNavSignature> {
);
}

// controls if the component reacts to viewport changes
get isResponsive(): boolean {
return this.args.isResponsive ?? true;
}

// controls if users can collapse the appsidenav on 'desktop' viewports
get isCollapsible(): boolean {
return this.args.isCollapsible ?? false;
}

get shouldTrapFocus(): boolean {
return this.isResponsive && !this.isDesktop && !this.isMinimized;
}
Expand Down
10 changes: 8 additions & 2 deletions packages/components/src/components/hds/table/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,13 +87,19 @@ export interface HdsTableSignature {
}

export default class HdsTable extends Component<HdsTableSignature> {
@tracked sortBy = this.args.sortBy ?? undefined;
@tracked sortOrder = this.args.sortOrder || HdsTableThSortOrderValues.Asc;
@tracked sortBy;
@tracked sortOrder;
@tracked selectAllCheckbox?: HdsFormCheckboxBaseSignature['Element'] =
undefined;
selectableRows: HdsTableSelectableRow[] = [];
@tracked isSelectAllCheckboxSelected?: boolean = undefined;

constructor(owner: unknown, args: HdsTableSignature['Args']) {
super(owner, args);
this.sortBy = this.args.sortBy ?? undefined;
this.sortOrder = this.args.sortOrder || HdsTableThSortOrderValues.Asc;
}

get getSortCriteria(): string | HdsTableSortingFunction<unknown> {
// get the current column
const currentColumn = this.args?.columns?.find(
Expand Down
8 changes: 6 additions & 2 deletions packages/components/src/components/hds/table/th-selectable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,13 +41,17 @@ export interface HdsTableThSelectableSignature {
}

export default class HdsTableThSelectable extends Component<HdsTableThSelectableSignature> {
@tracked isSelected = this.args.isSelected ?? false;

@tracked isSelected: boolean;
guid = guidFor(this);

checkboxId = `checkbox-${this.guid}`;
labelId = `label-${this.guid}`;

constructor(owner: unknown, args: HdsTableThSelectableSignature['Args']) {
super(owner, args);
this.isSelected = this.args.isSelected ?? false;
}

get isSortable(): boolean {
return this.args.onClickSortBySelected !== undefined;
}
Expand Down
3 changes: 2 additions & 1 deletion packages/components/src/components/hds/tabs/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ export default class HdsTabs extends Component<HdsTabsSignature> {
@tracked tabIds: HdsTabsTabIds = [];
@tracked panelNodes: HTMLElement[] = [];
@tracked panelIds: HdsTabsPanelIds = [];
@tracked _selectedTabIndex = this.args.selectedTabIndex ?? 0;
@tracked _selectedTabIndex;
@tracked selectedTabId?: string;
@tracked isControlled: boolean;

Expand Down Expand Up @@ -69,6 +69,7 @@ export default class HdsTabs extends Component<HdsTabsSignature> {

// this is to determine if the "selected" tab logic is controlled in the consumers' code or is maintained as an internal state
this.isControlled = this.args.selectedTabIndex !== undefined;
this._selectedTabIndex = this.args.selectedTabIndex ?? 0;
}

get selectedTabIndex(): number {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

import { module, test } from 'qunit';
import { setupRenderingTest } from 'ember-qunit';
import { click, render } from '@ember/test-helpers';
import { click, render, rerender } from '@ember/test-helpers';
import { hbs } from 'ember-cli-htmlbars';

module(
Expand Down Expand Up @@ -34,14 +34,22 @@ module(
assert.dom('.hds-form-visibility-toggle .hds-icon-eye').exists();
});

test('it should render readable text when `isContentMasked` is false', async function (assert) {
test('it should render readable text when `isContentMasked` is false and visibility should change if `isContentMasked` changes', async function (assert) {
this.set('isContentMasked', false);
await render(
hbs`<Hds::Form::MaskedInput::Base id="test-form-masked-input" @isContentMasked={{false}} />`
hbs`<Hds::Form::MaskedInput::Base id="test-form-masked-input" @isContentMasked={{this.isContentMasked}} />`
);
assert
.dom('.hds-form-masked-input__control')
.hasStyle({ '-webkit-text-security': 'none' });
assert.dom('.hds-form-visibility-toggle .hds-icon-eye-off').exists();

this.set('isContentMasked', true);
await rerender();
assert
.dom('.hds-form-masked-input__control')
.hasStyle({ '-webkit-text-security': 'disc' });
assert.dom('.hds-form-visibility-toggle .hds-icon-eye').exists();
});

test('it should toggle the masking when button is pressed', async function (assert) {
Expand Down
Loading
Loading