-
Notifications
You must be signed in to change notification settings - Fork 3.4k
Conversation
@marcysutton - as part of #340 [Refactor to use ngAria], we need to move these service methods to Utils and not use a custom $aria material service (since it conflicts). This also impacts #314. |
Ok. I figured the service would just be renamed. |
If we want to use
|
@ThomasBurleson any input on the timing issue mentioned above? |
@marcysutton - can I voip with you tomorrow AM after 11:00 AM CST? |
e582b55
to
94a233c
Compare
@@ -12,6 +12,8 @@ describe('material-slider', function() { | |||
}; | |||
} | |||
|
|||
// beforeEach(TestUtil.mockRaf); |
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.
Adding TestUtil.mockRaf
here causes the existing tests to fail. But without it, $materialAria.expect
does not run.
94a233c
to
828cb4e
Compare
var defaultValueTemplate = 'Default value was set: %s="%s".'; | ||
|
||
return { | ||
expect : expectAttribute, | ||
expect : $$rAF.debounce(expectAttribute), |
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.
@ajoslin @ThomasBurleson This causes an issue when there is more than one component of the same type on a page: $materialAria.expect
only executes for the last instance. Need to figure out a way to delay expectAttribute
from running but still execute for every component that needs it.
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.
Ah. right.
Just do:
function expectAttribute() {
$$rAF(function() {
//expectAttribute logic
});
}
bca0431
to
bad82be
Compare
element.attr(attrName, defaultValue); | ||
} else { | ||
// $log.warn(messageTemplate, attrName, getTagString(node)); | ||
function expectAttribute(element, attrName, copyElementText, defaultValue) { |
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.
Passing false
for copyElementText
will accommodate components requiring a warning for aria-label
if the developer forgot, such as material-slider
. Passing true
will try to copy the element's text (or use the optional defaultValue
, like in the case of material-dialog
). I'm open to renaming this parameter to something that sounds more boolean (or refactoring).
0615d2b
to
2ea9e3f
Compare
42c80f7
to
4c0c50e
Compare
2ea9e3f
to
063e61d
Compare
063e61d
to
a8cef8f
Compare
@@ -1,4 +1,4 @@ | |||
var DocsApp = angular.module('docsApp', ['ngMaterial', 'ngRoute', 'angularytics']) | |||
var DocsApp = angular.module('docsApp', ['ngMaterial', 'ngRoute', 'angularytics', 'ngAria']) |
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.
We include ngAria as a dependency of ngMaterial itself now, so this is extraneous.
a8cef8f
to
e6363c8
Compare
Merged via 3368c93. |
When a button, checkbox, radio button or slider is missing alternative text, warnings are logged to the developer telling them about the problem and directing them to the live node requiring a fix.
Closes #342