-
Notifications
You must be signed in to change notification settings - Fork 9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(elements-angular): directives matching multiple components #1215
Conversation
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.
Can you format this file (numeric-value-accessor.directive.ts
) according the others. The selector
value contains a whitespace.
Beside this the issue requires some test to prevent this issue in future. But I can´t see any 😄
@@ -5,7 +5,7 @@ import { ValueAccessorDirective } from './value-accessor.directive'; | |||
|
|||
@Directive({ | |||
selector: | |||
'ino-autocomplete,ino-currency-input,ino-input,ino-markdown-editor,ino-textarea,ino-range,ino-select,ino-datepicker,ino-segment-group', | |||
'ino-autocomplete,ino-currency-input,ino-input:not([type=number]):not([type=file]),ino-markdown-editor,ino-textarea,ino-select,ino-datepicker,ino-segment-group', |
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.
For using multiply :not()
selectors I suggest writing it like here:
https://developer.mozilla.org/en-US/blog/css-not-pseudo-multiple-selectors/#negating_multiple_selectors
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.
I've just tried to add some simple jest test to the package but I think as long as we've got these conflicts with stencils outdated Jest version, I don't see us adding any new tests soon. I'll write that down in ticket #1224.
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.
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.
Alright, then we should keep in mind that we write some tests later on.
Closes #1213
NumericValueAccessorDirective
as well asTextValueAccessorDirective
which led to runtime errors.