✨ FEATURE: AMP form dirtiness indicator class #22640
Conversation
This currently supports detecting the dirtiness of text-typed `<input>` and `<textarea>`. This does not take the submitted value into account yet.
| this.dirtyFieldCount_ = 0; | ||
|
|
||
| /** @private {!Object<string, boolean>} */ | ||
| this.isFieldNameDirty_ = {}; |
There was a problem hiding this comment.
Why we need a map rather than a Set()?
In that way we can also get rid of the counter.
There was a problem hiding this comment.
Right I should've left a comment. Set is not allowed here
amphtml/build-system/conformance-config.textproto
Lines 189 to 193 in e9abd3b
I think it's probably due to compatibility (#6551). I don't think we need to polyfill since its effects can be emulated with an object.
| this.dirtyFieldCount_ = 0; | ||
|
|
||
| /** @private {!Object<string, boolean>} */ | ||
| this.isFieldNameDirty_ = {}; |
There was a problem hiding this comment.
Let's use the src/object.js#map type here instead of object literals. It creates a new prototypeless object, which decreases the cost of property lookups under the hood.
this.isFieldNameDirty_ = map();There was a problem hiding this comment.
TIL {} vs Object.create thanks!
| * @private | ||
| */ | ||
| updateDirtinessClass_() { | ||
| if (this.dirtyFieldCount_ == 0 || this.isSubmitting_) { |
There was a problem hiding this comment.
This can be simplified into
const isDirty = this.dirtyFieldCount_ > 0 && !this.isSubmitting_;
this.form_.classList.toggle(DIRTINESS_INDICATOR_CLASS, isDirty);It creates a new prototypeless object, which decreases the cost of property lookups under the hood.
| }); | ||
| }); | ||
|
|
||
| describe('#onSubmitting', () => { |
There was a problem hiding this comment.
Just curious, what does this # stands for, the method name?
There was a problem hiding this comment.
Ahh an old habit. # denotes instance methods while . or :: are for static methods. I couldn't remember/find the source of this convention tho. Just grep'ed the AMP code base -
|
@cvializ ready now, thanks! |
* FEATURE: AMP form dirtiness indicator class
This currently supports detecting the dirtiness of text-typed `<input>`
and `<textarea>`. This does not take the submitted value into account
yet.
* remove incorrect `@const` annotation
* do not hook this up yet
* remove by keeping the property instead
* rename 'listeners' to 'handlers'
* move `updateDirtinessClass_` up
* use `utils/object#map` instead of `{}`
It creates a new prototypeless object, which decreases the cost of
property lookups under the hood.
* refactoring for simplicity and readability
* Added description for non-trivial methods
This currently supports detecting the dirtiness of text-typed
<input>and
<textarea>. This does not take the submitted value into accountyet.
This is part of #22534.
/cc @GoTcWang @cvializ