Skip to content
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

Merged
merged 1 commit into from
Sep 15, 2017

Conversation

ThomasBurleson
Copy link
Contributor

@ThomasBurleson ThomasBurleson commented Sep 13, 2017

  • 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, #420. Closes #418.

BREAKING CHANGES

  • ngStyle, ngClass should be used for responsive selectors and changes. Raw style and class attributes no longer support responsive suffices.
Before
<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>
Now
<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>

@ThomasBurleson
Copy link
Contributor Author

@mmalerba - ready for your review and g3 testing.

public hasResponsiveAPI() {
const totalKeys = Object.keys(this._inputMap).length;
const baseValue = this._inputMap[this._baseKey];
return (totalKeys - (!!baseValue ? 1 : 0)) > 0;
Copy link
Contributor

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)?

Copy link
Contributor Author

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 ^.

Copy link
Contributor Author

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.

Copy link
Contributor

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

@mmalerba mmalerba added the pr: lgtm This PR has been approved by the reviewer label Sep 14, 2017
value = this._ngClassAdapter.mqActivation.activatedInput;
protected _fallbackToKlass() {
if (!this._base.queryInput('ngClass')) {
this.ngClassBase = this._getAttributeValue('class');
Copy link
Contributor Author

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.

  1. Is this a valid assumption to fallback/restore the initial class value?

* 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>
```
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: yes pr: lgtm This PR has been approved by the reviewer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error "Can't bind to 'ngStyle.md' since it isn't a known property of 'div'"
3 participants