feat: autofill solution #599

gullerya · 2021-01-21T09:28:20Z

No description provided.

…l-solution

github-actions · 2021-01-21T09:28:36Z

🚀

Latest successful build of the PR deployed here.

🚀

yinonov · 2021-01-21T19:03:57Z

components/textfield/src/vwc-textfield.scss

+::slotted(input) {
+	position: absolute;
+	top: 0;
+	left: 0;
+	width: 100%;
+	height: 100%;
+	padding: 0 16px;
+	box-sizing: border-box;
+	border-radius: var(--border-radius);
+	border: none;
+	background: transparent;
+	outline: none;
+}


fiddling with input to inset fully results in overlapping icons. this can be alternated by attribute selectors, but I'd suggest leaving this indication to be handled by dedicated overlay element (as in ripple approach)

so you suggest to state with the current implementation (I mean the picture you show is not happening in the brach as it is, right)?

yinonov · 2021-01-22T07:58:04Z

ran into this trick couple of times 👉 https://github.com/matteobad/detect-autofill/blob/master/src/detect-autofill.js
interesting?

gullerya · 2021-01-22T08:12:38Z

Oh man, please no! Just compare the current solution - all works and almost zero code/css, and all those hack around! Why get ourselves in trouble?

…

On Fri, 22 Jan 2021, 9:58 yinon, ***@***.***> wrote: ran into this trick couple of times 👉 https://github.com/matteobad/detect-autofill/blob/master/src/detect-autofill.js interesting? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#599 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABG2SNPW3LBUGAVEVRUZ2X3S3EVZXANCNFSM4WMRCNLA> .

YonatanKra · 2021-01-24T07:57:53Z

components/textfield/src/vwc-textfield.ts

+		fe.onblur = this.onInputBlur.bind(this);
+
+		//	attributes/properties
+		fe.setAttribute('id', this.id);


Isn't this a bit problematic? We're going to have the same ID both on the custom-element as well as on the input.

I thought about this too, while doing that. Concern and my thoughts - sharing to put it here and then up to your take guys:

input element having id is somewhat important part of input (for label association, aria label association, sometimes seems to participate in form association and autofill)

id should be unique, this is pain

to transfer id from parent to child (remove on parent) is no go, since it's intervention into consumer's domain

duplicate id IS ugly, no brainer, but not breaking things and seemings won't harm

I'm actually inclined now to remove this and add when absolutely proven needed later.
WDYT?

You said that autofill takes id into account - but it also takes name, no?

Also, right, so presumable we can stay with name meanwhile. Just see this and pay attention to that wording: user again main require name and/or id attribute.

YonatanKra · 2021-01-24T07:59:23Z

components/textfield/src/vwc-textfield.ts

+	protected renderInput(): TemplateResult {
+		this.updateInputElement();
+		return html`
+			<div class="mdc-text-field__input"></div>


What's this for? Is it going to be used by someone?

We need it to preserve the effect of that mdc-text-field__input class - spacing, elements positioning around etc.
This is the simplest way to actually preserve all the MWC heavy lifting with that part.

…nto autofill-solution

…ll-solution # Conflicts: # common/foundation/test/autofocus.test.js

…nto autofill-solution

sonarcloud · 2021-01-24T21:13:17Z

SonarCloud Quality Gate failed.

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
27.4% Duplication

YonatanKra · 2021-01-25T09:04:34Z

components/textfield/src/vwc-textfield.ts

+		fe.onfocus = this.onInputFocus.bind(this);
+		fe.onblur = this.onInputBlur.bind(this);
+
+		//	attributes
+		setAttributeIfDefined('id', this.id, fe);
+		setAttributeIfDefined('name', this.name, fe);
+		setAttributeIfDefined('type', this.type, fe);
+		setAttributeIfDefined('form', this.form, fe);
+		setAttributeIfDefined('placeholder', this.placeholder, fe);
+
+		setAttributeIfDefined('min', this.min, fe);
+		setAttributeIfDefined('max', this.max, fe);
+		setAttributeIfDefined('step', this.step, fe);
+		setAttributeIfDefined('size', this.size, fe);
+
+		const autoCapOrNone = this.autocapitalize ? this.autocapitalize : undefined;
+		const minOrNone = this.minLength === -1 ? undefined : this.minLength;
+		const maxOrNone = this.maxLength === -1 ? undefined : this.maxLength;
+		setAttributeIfDefined('autocapitalize', autoCapOrNone, fe);
+		setAttributeIfDefined('minlength', minOrNone, fe);
+		setAttributeIfDefined('maxlength', maxOrNone, fe);
+		setAttributeIfDefined('pattern', this.pattern, fe);
+		setAttributeIfDefined('inputmode', this.inputMode, fe);
+
+		addAttributeByCondition('disabled', this.disabled, fe);
+		addAttributeByCondition('readonly', this.readOnly, fe);
+		addAttributeByCondition('required', this.required, fe);
+
+		const ariaLabel = shouldRenderHelperText ? 'helper-text' : undefined;
+		const ariaError =
+			this.validationMessage && !this.isUiValid ? 'helper-text' : undefined;
+		setAttributeIfDefined('aria-controls', ariaLabel, fe);
+		setAttributeIfDefined('aria-describedby', ariaLabel, fe);
+		setAttributeIfDefined('aria-errortext', ariaError, fe);


YonatanKra · 2021-01-25T09:06:36Z

components/textfield/test/textfield.test.js

-				const [formElement] = addElement(
-					createElementInForm(fieldName, fieldValue)
-				);
+				const [formElement] = createElementInForm(fieldName, fieldValue);


Suggested change

const [formElement] = createElementInForm(fieldName, fieldValue);

const [formElement] = addElement(createElementInForm(fieldName, fieldValue));

YonatanKra

LGTM. I'll push one small change

…l-solution

…nto autofill-solution

sonarcloud · 2021-01-25T10:04:20Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

YonatanKra and others added 7 commits January 18, 2021 19:35

Merge remote-tracking branch 'upstream/master' into autofill-solution

d82b033

Merge remote-tracking branch 'upstream/master' into autofill-solution

0d8f838

Return a slot on render

06afc14

Make the binding between input.value and this.value

9b7192e

Merge branch 'master' of https://github.com/Vonage/vivid into autofil…

d0c9516

…l-solution

some fixes to the event handling

1dfc868

fixed some styles, to see we are on trach - only positioning left

79781e7

gullerya marked this pull request as draft January 21, 2021 09:28

gullerya added 2 commits January 21, 2021 13:05

finalized CSS adjust

28582c5

last touches, removing that beige stuff

bce08b2

yinonov suggested changes Jan 21, 2021

View reviewed changes

yinonov added 2 commits January 23, 2021 10:25

style(textfiled): input padding - cover all cases

738812c

keep cursor's agent style sheet definition

9d6678e

YonatanKra reviewed Jan 24, 2021

View reviewed changes

gullerya requested review from YonatanKra and yinonov January 24, 2021 11:20

gullerya and others added 10 commits January 24, 2021 13:26

fixing the calendar snapshot

55b100a

fixing the tests: snapshot + typography

cf653bb

fixing the autofocus tests

d685e42

fixing autofocus tests - remove only and use addElement

d25d8d2

native input layout style coverage 90%

33ba5f7

Merge branch 'autofill-solution' of https://github.com/Vonage/vivid i…

9efc11e

…nto autofill-solution

Fix all except the reset form

b495640

Fix autofocus stuff

a262ef9

Merge remote-tracking branch 'upstream/autofill-solution' into autofi…

0611896

…ll-solution # Conflicts: # common/foundation/test/autofocus.test.js

fixing the reset?

d20badf

yinonov and others added 6 commits January 24, 2021 22:12

call layout after value change

1e227f7

Merge branch 'autofill-solution' of https://github.com/Vonage/vivid i…

ee7c2c2

…nto autofill-solution

duplicate selector

33931f0

Merge branch 'master' into autofill-solution

2687bcb

fixed the form story

742a061

Merge branch 'autofill-solution' of https://github.com/Vonage/vivid i…

49dc8c3

…nto autofill-solution

gullerya and others added 4 commits January 24, 2021 23:35

fixing the Sonar remarks

729f0c3

textfield with icons styling

77330d4

back the 14px to be aligned

cdc2ddf

adding last attributes and arrangin the clode

89154c8

gullerya marked this pull request as ready for review January 25, 2021 06:12

gullerya and others added 3 commits January 25, 2021 09:14

Merge branch 'master' into autofill-solution

813363d

z-index as autofill-only concern

45fc9a6

get background for autofill ready

4eaaa57

YonatanKra reviewed Jan 25, 2021

View reviewed changes

YonatanKra previously approved these changes Jan 25, 2021

View reviewed changes

Test elements cleanup

7ab2092

YonatanKra dismissed their stale review via 7ab2092 January 25, 2021 09:10

gullerya added 3 commits January 25, 2021 11:22

Merge branch 'master' into autofill-solution

f2af82f

fixing Yonatan remark

fe0b0bb

Merge branch 'master' of https://github.com/Vonage/vivid into autofil…

2c0d4ac

…l-solution

gullerya requested a review from YonatanKra January 25, 2021 09:28

gullerya added 2 commits January 25, 2021 11:28

Merge branch 'autofill-solution' of https://github.com/Vonage/vivid i…

792c1f9

…nto autofill-solution

Merge branch 'master' into autofill-solution

fd2b07d

yinonov approved these changes Jan 25, 2021

View reviewed changes

yinonov merged commit 284a5f0 into master Jan 25, 2021

gullerya deleted the autofill-solution branch January 25, 2021 11:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: autofill solution #599

feat: autofill solution #599

gullerya commented Jan 21, 2021

github-actions bot commented Jan 21, 2021

yinonov Jan 21, 2021

gullerya Jan 21, 2021

yinonov commented Jan 22, 2021

gullerya commented Jan 22, 2021 via email

YonatanKra Jan 24, 2021

gullerya Jan 24, 2021

YonatanKra Jan 24, 2021

gullerya Jan 24, 2021

YonatanKra Jan 24, 2021

gullerya Jan 24, 2021

sonarcloud bot commented Jan 24, 2021

YonatanKra Jan 25, 2021

YonatanKra Jan 25, 2021

gullerya Jan 25, 2021

YonatanKra left a comment

sonarcloud bot commented Jan 25, 2021

	const [formElement] = createElementInForm(fieldName, fieldValue);
	const [formElement] = addElement(createElementInForm(fieldName, fieldValue));

feat: autofill solution #599

feat: autofill solution #599

Conversation

gullerya commented Jan 21, 2021

github-actions bot commented Jan 21, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yinonov commented Jan 22, 2021

gullerya commented Jan 22, 2021 via email

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarcloud bot commented Jan 24, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

YonatanKra left a comment

Choose a reason for hiding this comment

sonarcloud bot commented Jan 25, 2021