fix(number-field): select full value when using Tab to enter a field with a unit #4340
Conversation
Lighthouse scores
What is this?Lighthouse scores comparing the documentation site built from the PR ("Branch") to that of the production documentation site ("Latest") and the build currently on Transfer Size
Request Count
|
Tachometer resultsChromecolor-field permalinkbasic-test
combobox permalinkbasic-test
light-dom-test permalink
number-field permalinkbasic-test
search permalinktest-basic
slider permalinktest-basic
textfield permalinktest-basic
Firefoxcolor-field permalinkbasic-test
combobox permalinkbasic-test
light-dom-test permalink
number-field permalinkbasic-test
search permalinktest-basic
slider permalinktest-basic
textfield permalinktest-basic
|
Westbrook
force-pushed
the
number-field-selection
branch
8 times, most recently
from
f462bcb
to
effd80f
Compare
Westbrook
force-pushed
the
number-field-selection
branch
4 times, most recently
from
ecbec82
to
7973c41
Compare
| @@ -355,6 +357,7 @@ export class TextfieldBase extends ManageHelpText( | |||
| .value=${live(this.displayValue)} | |||
| @change=${this.handleChange} | |||
| @input=${this.handleInput} | |||
| @pointerdown=${this.handleInputElementPointerdown} | |||
There was a problem hiding this comment.
Yeah, it's an annoyance of the class extension system and templating that this is required but the idea is definitely that by default Textfield does nothing for pointer down events and then in NumberField it does something specific. It implies the benefit of a alternative composition techniques within these render methods, but that's a huge alternative project with unclear benefits and risks.
|
|
||
| protected override handleInputElementPointerdown(): void { | ||
| this.hasRecentlyReceivedPointerDown = true; | ||
| this.updateComplete.then(() => { |
There was a problem hiding this comment.
Won't this cause a delay in the event to complete?
There was a problem hiding this comment.
This is a reaction to the event having been triggered. Because it is in the promise (updateComplete) it will release the thread during the event and wait till the promise releases before reengaging the workflow.
| !!this.formatOptions.unit | ||
| ) { | ||
| // Normalize keyboard focus entry between unit and non-unit bearing Number Fields | ||
| this.setSelectionRange(0, this.displayValue.length); |
There was a problem hiding this comment.
Do we need to check if displayValue has a length or not right?
There was a problem hiding this comment.
displayValue is always a string, and even if it's length is 0 the outcome here will still be correct.
| @@ -355,6 +357,7 @@ export class TextfieldBase extends ManageHelpText( | |||
| .value=${live(this.displayValue)} | |||
| @change=${this.handleChange} | |||
| @input=${this.handleInput} | |||
| @pointerdown=${this.handleInputElementPointerdown} | |||
There was a problem hiding this comment.
Yeah, it's an annoyance of the class extension system and templating that this is required but the idea is definitely that by default Textfield does nothing for pointer down events and then in NumberField it does something specific. It implies the benefit of a alternative composition techniques within these render methods, but that's a huge alternative project with unclear benefits and risks.
Westbrook
force-pushed
the
number-field-selection
branch
2 times, most recently
from
14387f6
to
851f1c4
Compare
Westbrook
force-pushed
the
number-field-selection
branch
from
851f1c4
to
d48c989
Compare
Westbrook
force-pushed
the
number-field-selection
branch
2 times, most recently
from
1b3aba5
to
65a3f30
Compare
Westbrook
force-pushed
the
number-field-selection
branch
from
65a3f30
to
422ea59
Compare
Westbrook
force-pushed
the
number-field-selection
branch
from
422ea59
to
1c643fe
Compare
Description
Keyboard based focus of Number Fields with units should select all of the available content.
Related issue(s)
How has this been tested?
Tabto focus the Number Field, see that all of the content is selectedTypes of changes
Checklist