-
Notifications
You must be signed in to change notification settings - Fork 2.1k
feat(textfield): helperTextContent setter #1569
Changes from 3 commits
3cfd6e9
ecdb071
a47b333
71fff53
76a58bf
4f45cdb
024fa61
00b48f9
335a394
b605a4d
960c3fe
d6af5c3
37c078d
af16d8d
dde368a
958a046
6a0d26d
f79f9b4
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 |
---|---|---|
|
@@ -113,6 +113,14 @@ class MDCTextfield extends MDCComponent { | |
this.foundation_.setValid(valid); | ||
} | ||
|
||
get helptextValue() { | ||
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. Remember to annotate for Closure: https://material.io/components/web/docs/closure-compiler/ 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. Good catch, thanks! |
||
return this.foundation_.getHelptextValue(); | ||
} | ||
|
||
set helptextValue(value) { | ||
this.foundation_.setHelptextValue(value); | ||
} | ||
|
||
/** | ||
* @return {!MDCTextfieldFoundation} | ||
*/ | ||
|
@@ -249,6 +257,17 @@ class MDCTextfield extends MDCComponent { | |
this.helptextElement.removeAttribute(name); | ||
} | ||
}, | ||
setHelptextValue: (value) => { | ||
if (this.helptextElement) { | ||
this.helptextElement.textContent = value; | ||
} | ||
}, | ||
getHelptextValue: () => { | ||
if (!this.helptextElement) { | ||
return; | ||
} | ||
return this.helptextElement.textContent.trim(); | ||
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. What's the reason for adding 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. Good point, I was trying to ensure that the returned value was close to what the user expected but that sounds like a false assumption on my part. For example:
would have a That being said, I see now that's an incorrect assumption. I'll remove |
||
}, | ||
}; | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -43,6 +43,7 @@ test('defaultAdapter returns a complete adapter implementation', () => { | |
'registerInputInteractionHandler', 'deregisterInputInteractionHandler', | ||
'registerTransitionEndHandler', 'deregisterTransitionEndHandler', | ||
'setBottomLineAttr', 'setHelptextAttr', 'removeHelptextAttr', 'getNativeInput', | ||
'setHelptextValue', 'getHelptextValue', | ||
]); | ||
}); | ||
|
||
|
@@ -173,6 +174,28 @@ test('#init does not add mdc-textfield__label--float-above class if the input do | |
td.verify(mockAdapter.addClassToLabel(cssClasses.LABEL_FLOAT_ABOVE), {times: 0}); | ||
}); | ||
|
||
test('#setHelptextValue has no effect when no help text element is present', () => { | ||
const {foundation} = setupTest(); | ||
foundation.setHelptextValue('foo'); | ||
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. Hmmmm... I wonder if there's a way to make this a more isolated and specific test. It's not really testing what we're saying it's testing. 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. Good point. I moved that test to the |
||
}); | ||
|
||
test('#setHelptextValue sets the value of the help text element', () => { | ||
const {foundation, mockAdapter} = setupTest(); | ||
foundation.setHelptextValue('foo'); | ||
td.verify(mockAdapter.setHelptextValue('foo')); | ||
}); | ||
|
||
test('#getHelptextValue has no effect when no help text element is present', () => { | ||
const {foundation} = setupTest(); | ||
foundation.getHelptextValue(); | ||
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. Same comment as above 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. Same as above, moved it to the |
||
}); | ||
|
||
test('#getHelptextValue retrieves the value of the help text element', () => { | ||
const {foundation, mockAdapter} = setupTest(); | ||
foundation.getHelptextValue(); | ||
td.verify(mockAdapter.getHelptextValue()); | ||
}); | ||
|
||
test('on input focuses if input event occurs without any other events', () => { | ||
const {foundation, mockAdapter} = setupTest(); | ||
let input; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -131,6 +131,22 @@ test('set valid updates the component styles', () => { | |
assert.isNotOk(root.classList.contains(cssClasses.INVALID)); | ||
}); | ||
|
||
test('set helptextValue updates the help text element content', () => { | ||
const {component} = setupTest(); | ||
const helptext = getHelptext(); | ||
component.helptextElement = helptext; | ||
component.helptextValue = 'foo'; | ||
assert.equal(helptext.textContent, 'foo'); | ||
}); | ||
|
||
test('get helptextValue retrieves the help text element content', () => { | ||
const {component} = setupTest(); | ||
const helptext = getHelptext(); | ||
component.helptextElement = helptext; | ||
helptext.textContent = 'foo'; | ||
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. I don't think we need to set anything here first, right? The fixture has a value already. 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. Ah, good find. I'll switch the check for the fixture's default value. Thanks! |
||
assert.equal(component.helptextValue, 'foo'); | ||
}); | ||
|
||
test('#adapter.addClassToBottomLine adds a class to the bottom line', () => { | ||
const {bottomLine, component} = setupTest(); | ||
component.getDefaultFoundation().adapter_.addClassToBottomLine('foo'); | ||
|
@@ -330,6 +346,32 @@ test('#adapter.removeHelptextAttr removes an attribute on the help text element' | |
assert.isNotOk(helptext.hasAttribute('aria-label')); | ||
}); | ||
|
||
test('#adapter.setHelptextValue does nothing if no help text element present', () => { | ||
const {component} = setupTest(); | ||
assert.doesNotThrow(() => component.getDefaultFoundation().adapter_.setHelptextValue('foo')); | ||
}); | ||
|
||
test('#adapter.setHelptextValue updates the text content of the help text element', () => { | ||
const {component} = setupTest(); | ||
const helptext = getHelptext(); | ||
component.helptextElement = helptext; | ||
component.getDefaultFoundation().adapter_.setHelptextValue('foo'); | ||
assert.equal(helptext.textContent, 'foo'); | ||
}); | ||
|
||
test('#adapter.getHelptextValue does nothing if no help text element present', () => { | ||
const {component} = setupTest(); | ||
assert.doesNotThrow(() => component.getDefaultFoundation().adapter_.getHelptextValue()); | ||
}); | ||
|
||
test('#adapter.getHelptextValue retrieves the text content of the help text element', () => { | ||
const {component} = setupTest(); | ||
const helptext = getHelptext(); | ||
component.helptextElement = helptext; | ||
component.getDefaultFoundation().adapter_.setHelptextValue('foo'); | ||
assert.equal(component.getDefaultFoundation().adapter_.getHelptextValue(), 'foo'); | ||
}); | ||
|
||
test(`#adapter.notifyIconAction emits ${strings.ICON_EVENT}`, () => { | ||
const {component} = setupTest(); | ||
const handler = td.func('leadingHandler'); | ||
|
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.
nit: should read
getHelptextValue
/setHelptextValue
I realize you were just following the pattern for
MDCTextfield.disabled
, but that should probably change too.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.
Easy fix, thanks!