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

Error text overflow #2432

Draft
wants to merge 2 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
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
"comment": "Update error text on components to only show a title when the error text overflows",
"packageName": "@ni/nimble-components",
"email": "20542556+mollykreis@users.noreply.github.com",
"dependentChangeType": "patch"
}
4 changes: 4 additions & 0 deletions packages/nimble-components/src/combobox/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,10 @@ export class Combobox
@observable
public hasOverflow = false;

/* @internal */
@observable
public errorHasOverflow = false;

public override get value(): string {
Observable.track(this, 'value');
return this._value;
Expand Down
6 changes: 5 additions & 1 deletion packages/nimble-components/src/number-field/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { attr, html } from '@microsoft/fast-element';
import { attr, html, observable } from '@microsoft/fast-element';
import {
DesignSystem,
NumberField as FoundationNumberField,
Expand Down Expand Up @@ -44,6 +44,10 @@ export class NumberField extends FoundationNumberField implements ErrorPattern {
@attr({ attribute: 'error-visible', mode: 'boolean' })
public errorVisible = false;

/* @internal */
@observable
public errorHasOverflow = false;

public override connectedCallback(): void {
super.connectedCallback();

Expand Down
8 changes: 7 additions & 1 deletion packages/nimble-components/src/patterns/error/template.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,14 @@
import { html } from '@microsoft/fast-element';
import type { ErrorPattern } from './types';
import { overflow } from '../../utilities/directive/overflow';

export const errorTextTemplate = html<ErrorPattern>`
<div class="error-text" title="${x => x.errorText}" aria-live="polite">
<div
class="error-text"
${overflow('errorHasOverflow')}
title="${x => (x.errorHasOverflow && x.errorText ? x.errorText : null)}"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably worthwhile to write unit tests for this behavior against one (not all) of the components that use this pattern. I'm pretty sure we have some existing title overflow behavior tests for labels, etc.

aria-live="polite"
>
${x => x.errorText}
</div>
`;
7 changes: 7 additions & 0 deletions packages/nimble-components/src/patterns/error/types.ts
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could/should we replace the ErrorPattern interface with a mixin? Would be nice to remove all the declarations of those three properties and automatically enforce the @attr and @observable expectations.

Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,11 @@ export interface ErrorPattern {
* Show the error appearance of the control
*/
errorVisible: boolean;

/* @internal
* Indicates if the error text has overflowed its container. The value should not be
* set directly. Instead, it is used with the `overflow` directive. When declared in an
* implementation of `ErrorPattern`, it must be declared as `@observable`.
*/
errorHasOverflow: boolean;
}
6 changes: 5 additions & 1 deletion packages/nimble-components/src/radio-group/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import {
DesignSystem
} from '@microsoft/fast-foundation';
import { Orientation } from '@microsoft/fast-web-utilities';
import { attr } from '@microsoft/fast-element';
import { attr, observable } from '@microsoft/fast-element';
import { styles } from './styles';
import { template } from './template';
import type { ErrorPattern } from '../patterns/error/types';
Expand All @@ -25,6 +25,10 @@ export class RadioGroup extends FoundationRadioGroup implements ErrorPattern {

@attr({ attribute: 'error-visible', mode: 'boolean' })
public errorVisible = false;

/* @internal */
@observable
public errorHasOverflow = false;
}

const nimbleRadioGroup = RadioGroup.compose({
Expand Down
4 changes: 4 additions & 0 deletions packages/nimble-components/src/rich-text/editor/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,10 @@ export class RichTextEditor extends RichText implements ErrorPattern {
*/
public editorContainer!: HTMLDivElement;

/* @internal */
@observable
public errorHasOverflow = false;

private resizeObserver?: ResizeObserver;
private updateScrollbarWidthQueued = false;

Expand Down
4 changes: 4 additions & 0 deletions packages/nimble-components/src/select/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,10 @@ export class Select
return slotTextContent(this.labelSlot);
}

/* @internal */
@observable
public errorHasOverflow = false;

private _value = '';
private forcedPosition = false;
private openActiveIndex?: number;
Expand Down
4 changes: 4 additions & 0 deletions packages/nimble-components/src/text-area/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,10 @@ export class TextArea extends FoundationTextArea implements ErrorPattern {
@observable
public scrollbarWidth = -1;

/* @internal */
@observable
public errorHasOverflow = false;

private resizeObserver?: ResizeObserver;
private updateScrollbarWidthQueued = false;

Expand Down
6 changes: 5 additions & 1 deletion packages/nimble-components/src/text-field/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { attr, html } from '@microsoft/fast-element';
import { attr, html, observable } from '@microsoft/fast-element';
import {
DesignSystem,
TextField as FoundationTextField,
Expand Down Expand Up @@ -46,6 +46,10 @@ export class TextField extends FoundationTextField implements ErrorPattern {

@attr({ attribute: 'full-bleed', mode: 'boolean' })
public fullBleed = false;

/* @internal */
@observable
public errorHasOverflow = false;
}

const nimbleTextField = TextField.compose<TextFieldOptions>({
Expand Down