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(flexbox): resolve 'renderer.setStyle()' error #298

Merged
merged 1 commit into from
May 25, 2017

Conversation

ThomasBurleson
Copy link
Contributor

@ThomasBurleson ThomasBurleson commented May 25, 2017

  • Revert back to Renderer (from Renderer2)

injecting both Renderers causes intermittent DI issues that manifest errors such as TypeError: _this._renderer.setStyle is not a function.

  • Refactor the FxBaseDirective to use a StyleRenderer interface

Fixes #270

@ThomasBurleson
Copy link
Contributor Author

Fixes #270 (comment)

screen shot 2017-05-25 at 11 14 17 am

@ThomasBurleson ThomasBurleson added pr: needs presubmit P0 Critical issue that needs to be resolved immediately and removed in-progress labels May 25, 2017
Copy link
Member

@crisbeto crisbeto left a comment

Choose a reason for hiding this comment

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

LGTM, one minor issue.

* To avoid dependencies on full class Renderer or Renderer,
* define interface for the only method used...
*/
export interface StyleRenderer {
Copy link
Member

Choose a reason for hiding this comment

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

You can get rid of this one and replace all the usages with the Renderer.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since the BaseFxDirective directive only uses the method setElementStyle() using a StyleRenderer interface clearly declares the intent of the reference.

@ThomasBurleson
Copy link
Contributor Author

ThomasBurleson commented May 25, 2017

History

Flex-Layout (beta.8) expects a Renderer2 with function ::setStyle() to injected.
Using M2 (beta.5) with the following setup, a Renderer::setElementStyle() is incorrectly injected.

Here is a Plunkr Demo

    <md-tab-group>
      <md-tab>
          <ng-template md-tab-label>
            <span fxShow>tab 1 with fxShow</span>
          </ng-template>
          <md-card fxFlex>
            <md-card-header class="md-primary">
              <b>social identity</b>
            </md-card-header>
            <md-card-content>
              hello world 1
            </md-card-content>
          </md-card>
        </md-tab>
      
        <md-tab label="Password" *ngIf='true'>
          <md-card fxFlex>
            <md-card-header class="md-primary">
              <b>Local identity</b>
            </md-card-header>
            <md-card-content>
              hello world 2
            </md-card-content>
          </md-card>
        </md-tab>
    </md-tab-group>

screen shot 2017-05-20 at 1 30 33 pm

When the fxShow used inside the <ng-template> is removed, then errors no longer manifest.

* Revert back to Renderer (from Renderer2); Injecting both Renderers
causes intermittent `TypeError: _this._renderer.setStyle is not a function` errors.
*  Refactor the FxBaseDirective to use a StyleRenderer interface

Fixes #270
@tmburnell
Copy link

Do we have any idea when beta.9 will be cut, so the update will be applied to the NPM version?

@ThomasBurleson
Copy link
Contributor Author

The plan is the end-of next week!

@ThomasBurleson ThomasBurleson deleted the thomas/issue-270 branch August 7, 2017 22:36
@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 P0 Critical issue that needs to be resolved immediately pr: needs presubmit
Projects
None yet
Development

Successfully merging this pull request may close these issues.

crash with 2.0.0-beta.8
5 participants