-
Notifications
You must be signed in to change notification settings - Fork 25
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
feat: input-date: Disclose when date was reverted to previous #5164
Changes from 15 commits
309b2ab
89b57bc
ba0cb79
7db278d
521b16d
d9ae386
1476740
fde7057
1974bbd
47c691c
d2af075
0ed12b7
f292c30
da33381
52b1342
ffe4986
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -89,7 +89,9 @@ class InputDate extends FocusMixin(LabelledMixin(SkeletonMixin(FormElementMixin( | |
_formattedValue: { type: String }, | ||
_inputTextFocusShowTooltip: { type: Boolean }, | ||
_showInfoTooltip: { type: Boolean }, | ||
_shownValue: { type: String } | ||
_shownValue: { type: String }, | ||
_showRevertInstructions: { state: true }, | ||
_showRevertTooltip: { state: true } | ||
}; | ||
} | ||
|
||
|
@@ -159,10 +161,11 @@ class InputDate extends FocusMixin(LabelledMixin(SkeletonMixin(FormElementMixin( | |
this._inputId = getUniqueId(); | ||
this._inputTextFocusMouseup = false; | ||
this._inputTextFocusShowTooltip = true; // true by default so hover triggers tooltip | ||
this._namespace = 'components.input-date'; | ||
this._openCalendarOnly = false; | ||
this._openedOnKeydown = false; | ||
this._showInfoTooltip = true; | ||
this._showRevertInstructions = false; | ||
this._showRevertTooltip = false; | ||
this._shownValue = ''; | ||
|
||
this._dateTimeDescriptor = getDateTimeDescriptor(); | ||
|
@@ -183,11 +186,11 @@ class InputDate extends FocusMixin(LabelledMixin(SkeletonMixin(FormElementMixin( | |
const minDate = this.minValue ? formatDate(getDateFromISODate(this.minValue), { format: 'medium' }) : null; | ||
const maxDate = this.maxValue ? formatDate(getDateFromISODate(this.maxValue), { format: 'medium' }) : null; | ||
if (minDate && maxDate) { | ||
return this.localize(`${this._namespace}.errorOutsideRange`, { minDate, maxDate }); | ||
return this.localize('components.input-date.errorOutsideRange', { minDate, maxDate }); | ||
} else if (maxDate) { | ||
return this.localize(`${this._namespace}.errorMaxDateOnly`, { maxDate }); | ||
return this.localize('components.input-date.errorMaxDateOnly', { maxDate }); | ||
} else if (this.minValue) { | ||
return this.localize(`${this._namespace}.errorMinDateOnly`, { minDate }); | ||
return this.localize('components.input-date.errorMinDateOnly', { minDate }); | ||
} | ||
} | ||
return super.validationMessage; | ||
|
@@ -237,12 +240,17 @@ class InputDate extends FocusMixin(LabelledMixin(SkeletonMixin(FormElementMixin( | |
const formattedWideDate = formatISODateInUserCalDescriptor('2323-12-23'); | ||
const shortDateFormat = (this._dateTimeDescriptor.formats.dateFormats.short).toUpperCase(); | ||
|
||
const clearButton = !this.required ? html`<d2l-button-subtle text="${this.localize(`${this._namespace}.clear`)}" @click="${this._handleClear}"></d2l-button-subtle>` : null; | ||
const nowButton = this.hasNow ? html`<d2l-button-subtle text="${this.localize(`${this._namespace}.now`)}" @click="${this._handleSetToNow}"></d2l-button-subtle>` : null; | ||
const clearButton = !this.required ? html`<d2l-button-subtle text="${this.localize('components.input-date.clear')}" @click="${this._handleClear}"></d2l-button-subtle>` : null; | ||
const nowButton = this.hasNow ? html`<d2l-button-subtle text="${this.localize('components.input-date.now')}" @click="${this._handleSetToNow}"></d2l-button-subtle>` : null; | ||
const icon = (this.invalid || this.childErrors.size > 0) | ||
? html`<d2l-icon icon="tier1:alert" slot="left" style="${styleMap({ color: 'var(--d2l-color-cinnabar)' })}"></d2l-icon>` | ||
: html`<d2l-icon icon="tier1:calendar" slot="left"></d2l-icon>`; | ||
const errorTooltip = (this.validationError && !this.opened && this.childErrors.size === 0) ? html`<d2l-tooltip align="start" announced for="${this._inputId}" state="error" class="vdiff-target">${this.validationError}</d2l-tooltip>` : null; | ||
|
||
const tooltip = this._getErrorTooltip() || this._getRevertTooltip(shortDateFormat); | ||
|
||
const inputTextInstructions = (this._showInfoTooltip && !tooltip && !this.invalid && this.childErrors.size === 0 && this._inputTextFocusShowTooltip) | ||
? this.localize('components.input-date.openInstructions', { format: shortDateFormat }) | ||
: undefined; | ||
|
||
const dropdownContent = this._dropdownFirstOpened ? html` | ||
<d2l-dropdown-content | ||
|
@@ -265,7 +273,7 @@ class InputDate extends FocusMixin(LabelledMixin(SkeletonMixin(FormElementMixin( | |
min-value="${ifDefined(this.minValue)}" | ||
selected-value="${ifDefined(this._shownValue)}"> | ||
<div class="d2l-calendar-slot-buttons"> | ||
<d2l-button-subtle text="${this.localize(`${this._namespace}.today`)}" @click="${this._handleSetToToday}"></d2l-button-subtle> | ||
<d2l-button-subtle text="${this.localize('components.input-date.today')}" @click="${this._handleSetToToday}"></d2l-button-subtle> | ||
${nowButton} | ||
${clearButton} | ||
</div> | ||
|
@@ -276,15 +284,15 @@ class InputDate extends FocusMixin(LabelledMixin(SkeletonMixin(FormElementMixin( | |
<div>${formattedWideDate}</div> | ||
<div>${shortDateFormat}</div> | ||
</div> | ||
${errorTooltip} | ||
${tooltip} | ||
<d2l-dropdown ?disabled="${this.disabled || this.skeleton}" no-auto-open ?prefer-fixed-positioning="${this.preferFixedPositioning}"> | ||
<d2l-input-text | ||
?novalidate="${this.noValidate}" | ||
aria-invalid="${this.invalid ? 'true' : 'false'}" | ||
@blur="${this._handleInputTextBlur}" | ||
@change="${this._handleChange}" | ||
class="d2l-dropdown-opener vdiff-target" | ||
instructions="${ifDefined((this._showInfoTooltip && !errorTooltip && !this.invalid && this.childErrors.size === 0 && this._inputTextFocusShowTooltip) ? this.localize(`${this._namespace}.openInstructions`, { format: shortDateFormat }) : undefined)}" | ||
instructions="${ifDefined(inputTextInstructions)}" | ||
?disabled="${this.disabled}" | ||
@focus="${this._handleInputTextFocus}" | ||
@keydown="${this._handleKeydown}" | ||
|
@@ -349,15 +357,62 @@ class InputDate extends FocusMixin(LabelledMixin(SkeletonMixin(FormElementMixin( | |
this._dropdown.close(); | ||
} | ||
|
||
_getErrorTooltip() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could also probably combine this with |
||
if (!this.validationError || this.opened || this.childErrors.size > 0) return null; | ||
|
||
const message = (this._showRevertTooltip || this._showRevertInstructions) | ||
? html` | ||
<div>${this.localize('components.input-date.revert', { label: this.label })}</div> | ||
<br> | ||
<div>${this.validationError}</div>` | ||
: this.validationError; | ||
|
||
return html` | ||
<d2l-tooltip | ||
align="start" | ||
announced | ||
class="vdiff-target" | ||
for="${this._inputId}" | ||
?force-show="${this._showRevertTooltip}" | ||
state="error"> | ||
${message} | ||
</d2l-tooltip> | ||
`; | ||
} | ||
|
||
_getRevertTooltip(shortDateFormat) { | ||
if (this.opened || this.invalid || (!this._showRevertInstructions && !this._showRevertTooltip)) return null; | ||
|
||
const revertMessage = this.localize('components.input-date.revert', { label: this.label }); | ||
const secondaryMessage = this._showRevertInstructions | ||
? this.localize('components.input-date.openInstructions', { format: shortDateFormat }) | ||
: this.localize('components.input-date.useDateFormat', { format: shortDateFormat }); | ||
|
||
return html` | ||
<d2l-tooltip | ||
align="start" | ||
announced | ||
class="vdiff-target" | ||
for="${this._inputId}" | ||
?force-show="${this._showRevertTooltip}"> | ||
<div>${revertMessage}</div> | ||
<br> | ||
<div>${secondaryMessage}</div> | ||
</d2l-tooltip> | ||
`; | ||
} | ||
|
||
_handleBlur() { | ||
this._showInfoTooltip = true; | ||
this.requestValidate(true); | ||
} | ||
|
||
async _handleChange() { | ||
this._showRevertTooltip = false; | ||
const value = this._textInput.value; | ||
const isNewVal = value !== this.value; | ||
if (!value && !this.required) { | ||
if (value !== this.value) { | ||
if (isNewVal) { | ||
await this._updateValueDispatchEvent(''); | ||
if (this._calendar) { | ||
await this.updateComplete; | ||
|
@@ -373,6 +428,7 @@ class InputDate extends FocusMixin(LabelledMixin(SkeletonMixin(FormElementMixin( | |
await this._updateValueDispatchEvent(formatDateInISO({ year: date.getFullYear(), month: (parseInt(date.getMonth()) + 1), date: date.getDate() })); | ||
} catch { | ||
// leave value the same when invalid input | ||
if (isNewVal) this._showRevertTooltip = true; | ||
} | ||
this._setFormattedValue(); // keep out here in case parseDate is same date, e.g., user adds invalid text to end of parseable date | ||
if (this._calendar) { | ||
|
@@ -390,6 +446,7 @@ class InputDate extends FocusMixin(LabelledMixin(SkeletonMixin(FormElementMixin( | |
} | ||
|
||
async _handleDateSelected(e) { | ||
this._showRevertTooltip = false; | ||
const value = e.target.selectedValue; | ||
await this._updateValueDispatchEvent(value); | ||
if (this._dropdown) { | ||
|
@@ -448,9 +505,14 @@ class InputDate extends FocusMixin(LabelledMixin(SkeletonMixin(FormElementMixin( | |
_handleInputTextBlur() { | ||
this._inputTextFocusMouseup = false; | ||
this._inputTextFocusShowTooltip = true; | ||
this._showRevertInstructions = false; | ||
} | ||
|
||
_handleInputTextFocus() { | ||
if (this._showRevertTooltip) { | ||
this._showRevertInstructions = true; | ||
this._showRevertTooltip = false; | ||
} | ||
this._formattedValue = this._shownValue ? formatISODateInUserCalDescriptor(this._shownValue) : ''; | ||
|
||
// hide tooltip when focus, wait to see if click happened, then show | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
import '../input-date.js'; | ||
import { clickElem, expect, fixture, focusElem, html, nextFrame, oneEvent, sendKeys, sendKeysElem } from '@brightspace-ui/testing'; | ||
import { aTimeout, clickElem, expect, fixture, focusElem, html, nextFrame, oneEvent, sendKeys, sendKeysElem } from '@brightspace-ui/testing'; | ||
import { reset, useFakeTimers } from 'sinon'; | ||
import { ifDefined } from 'lit/directives/if-defined.js'; | ||
import { inlineHelpFixtures } from './input-shared-content.js'; | ||
|
@@ -351,6 +351,80 @@ describe('d2l-input-date', () => { | |
await sendKeysElem(elem, 'press', 'Enter'); | ||
await expect(elem).to.be.golden(); | ||
}); | ||
|
||
it('click then click away', async() => { | ||
await clickElem(elem); | ||
await clickElem(document.body); | ||
await expect(elem).to.be.golden(); | ||
}); | ||
|
||
it('open then close', async() => { | ||
await sendKeysElem(elem, 'press', 'Enter'); | ||
sendKeys('press', 'Escape'); | ||
await oneEvent(elem, 'd2l-tooltip-show'); | ||
await expect(elem).to.be.golden(); | ||
}); | ||
}); | ||
|
||
describe('required min-max revert', () => { | ||
|
||
let elem; | ||
beforeEach(async() => { | ||
elem = await fixture(create({ label: 'Date', labelHidden: false, required: true, value: '2012-01-01', maxValue: '2018-02-27', minValue: '2018-02-13' })); | ||
await focusElem(elem); | ||
await sendKeysElem(elem, 'press', 'Tab'); // trigger validation | ||
}); | ||
|
||
it('delete text input then blur', async() => { | ||
margaree marked this conversation as resolved.
Show resolved
Hide resolved
|
||
await focusElem(elem); | ||
await sendKeysElem(elem, 'press', 'Backspace'); | ||
await sendKeys('press', 'Tab'); | ||
await expect(elem).to.be.golden(); | ||
}); | ||
|
||
it('delete text input then blur then re-focus', async() => { | ||
await focusElem(elem); | ||
await sendKeysElem(elem, 'press', 'Backspace'); | ||
await sendKeys('press', 'Tab'); | ||
await focusElem(elem); | ||
await aTimeout(100); | ||
await expect(elem).to.be.golden(); | ||
}); | ||
|
||
}); | ||
|
||
describe('required revert', () => { | ||
|
||
let elem; | ||
beforeEach(async() => { | ||
elem = await fixture(create({ label: 'Date', labelHidden: false, required: true, value: '2020-01-01' })); | ||
}); | ||
|
||
it('delete text input then blur', async() => { | ||
await focusElem(elem); | ||
await sendKeysElem(elem, 'press', 'Backspace'); | ||
await sendKeys('press', 'Tab'); | ||
await expect(elem).to.be.golden(); | ||
}); | ||
|
||
it('delete text input then blur then re-focus', async() => { | ||
await focusElem(elem); | ||
await sendKeysElem(elem, 'press', 'Backspace'); | ||
await sendKeys('press', 'Tab'); | ||
await focusElem(elem); | ||
await aTimeout(1000); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So the problem that seems to be happening here is that in the blurred state the tooltip is less wide because it has less wide content, but then when we re-focus the content is wider. Testing this, it always ends up going to the wider width. However, in the vdiffs sometimes the screenshot is taken before it's gotten wider, leading to flake. I can do something like `await waitUntil(this.shadowRoot.querySelector('d2l-tooltip').shadowRoot.querySelector('...content div').clientWidth > 300, '...') but that feels bad. Are there alternatives I'm missing? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh wow, that tooltip is just permanent eh. The only other thing I can think of would be to just put the input into the state you want initially by setting |
||
await expect(elem).to.be.golden(); | ||
}); | ||
|
||
it('delete text input then blur then re-focus then blur', async() => { | ||
await focusElem(elem); | ||
await sendKeysElem(elem, 'press', 'Backspace'); | ||
await sendKeys('press', 'Tab'); | ||
await focusElem(elem); | ||
await sendKeys('press', 'Tab'); | ||
await aTimeout(100); | ||
await expect(elem).to.be.golden(); | ||
}); | ||
}); | ||
}); | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This uses
this._namespace
for the lang terms which had come up as being bad practice since it makes searching for lang terms more difficult.Since I was adding more lang term usage I thought I'd clean this up, but let me know if I shouldn't actually do this or if it's too much noise.