-
Notifications
You must be signed in to change notification settings - Fork 772
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(api, class, style): remove deprecated selectors #419
Conversation
fb17607
to
c36ea03
Compare
@mmalerba - ready for your review and g3 testing. |
src/lib/api/core/base-adapter.ts
Outdated
public hasResponsiveAPI() { | ||
const totalKeys = Object.keys(this._inputMap).length; | ||
const baseValue = this._inputMap[this._baseKey]; | ||
return (totalKeys - (!!baseValue ? 1 : 0)) > 0; |
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 this just be return this.hasResponsiveAPI(this._baseKey)
?
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.
Sorry @mmalerba - I do not understand your question here ^.
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.
the _baseKey
is the only non-responsive key. We want to check if 1 or more responsive keys are defined/in-use.
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'm just saying that this class extends a class with a 1-parameter method of the same name, it looks like this no-parameter version could just call the 1-parameter version
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.
Fixed.
c36ea03
to
fc8151d
Compare
src/lib/api/ext/class.ts
Outdated
value = this._ngClassAdapter.mqActivation.activatedInput; | ||
protected _fallbackToKlass() { | ||
if (!this._base.queryInput('ngClass')) { | ||
this.ngClassBase = this._getAttributeValue('class'); |
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.
Premise: When a breakpoint deactivates, the fallback value in the non-responsive input will be used.
But what happens when the fallback/default input is not defined?
Consider this logic here in _fallbackToKlass()
. Is this logic valid?
This methods effectively says that if a responsive breakpoint deactivates (e.g. [ngClass.md]
) and the default [ngClass]
has not been defined, then we should use the class
attribute value (if any) as the fallback value.
The challenge is the any ngClass
responsive input could be a string, array, or map. Each do different things.
- Is this a valid assumption to fallback/restore the initial
class
value?
fc8151d
to
24ec66f
Compare
* all [style], [style.xxx], [class], and [class.xxx] selectors have been removed * only ngStyle and ngClass selectors support responsive suffices * now use of raw `style` attribute will NOT instantiate a StyleDirective * now use of raw `class` attribute will NOT instantiate a ClassDirective * The `ClassDirective` now simply adds responsive enhancements to the core `NgClass` directive * The `StyleDirective` now simply adds responsive enhancements to the core `NgStyle` directive * Added missing [ngStyle.md] selector * Added `isBrowser()` universal checks for `getAttribute()` and `getStyle()` queries Fixes #410, #408, #273. Closes #418. BREAKING CHANGES * **ngStyle**, **ngClass** should be used for responsive selectors and changes. Raw `style` and `class` attributes no longer support responsive suffices. ```html <div fxLayout [class.xs]="['xs-1', 'xs-2']" [style]="{'font-size': '10px', 'margin-left' : '13px'}" [style.xs]="{'font-size': '16px'}" [style.md]="{'font-size': '12px'}"> </div> ``` ```html <div fxLayout [ngClass.xs]="['xs-1', 'xs-2']" [ngStyle]="{'font-size': '10px', 'margin-left' : '13px'}" [ngStyle.xs]="{'font-size': '16px'}" [ngStyle.md]="{'font-size': '12px'}"> </div> ```
24ec66f
to
5bf0146
Compare
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
[style]
,[style.xxx]
,[class]
, and[class.xxx]
selectors have been removedngStyle
andngClass
selectors support responsive sufficesstyle
attribute will NOT instantiate a StyleDirectiveclass
attribute will NOT instantiate a ClassDirectiveClassDirective
now simply adds responsive enhancements to the coreNgClass
directiveStyleDirective
now simply adds responsive enhancements to the coreNgStyle
directive[ngStyle.md]
selectorisBrowser()
universal checks forgetAttribute()
andgetStyle()
queriesFixes #410, #408, #273, #420. Closes #418.
BREAKING CHANGES
style
andclass
attributes no longer support responsive suffices.Before
Now