Skip to content
This repository has been archived by the owner on Jan 6, 2025. It is now read-only.

Fallback for fxLayoutGap is missing #1011

Closed
silvioboehme opened this issue Jan 29, 2019 · 7 comments · Fixed by #1037
Closed

Fallback for fxLayoutGap is missing #1011

silvioboehme opened this issue Jan 29, 2019 · 7 comments · Fixed by #1037
Assignees
Labels
bug has pr A PR has been created to address this issue P1 Urgent issue that should be resolved before the next re-lease
Milestone

Comments

@silvioboehme
Copy link

Bug Report

image

<div fxLayout="row" class="sec1" fxLayoutGap.gt-sm="50px">

If you reload the page at SM breakpoint, the gap is set to 0px. Now increase the window to MD, the gap is set to 50px, which is fine. But now, if you decrease the window to MD breakpoint, the gap is still 50px, and in the console you will get this error:

Uncaught TypeError: Cannot read property 'endsWith' of undefined at LayoutGapStyleBuilder.push../node_modules/@angular/flex-layout/esm5/flex.es5.js.LayoutGapStyleBuilder.buildStyles (flex.es5.js:228) at DefaultLayoutGapDirective.push../node_modules/@angular/flex-layout/esm5/core.es5.js.BaseDirective2.addStyles (core.es5.js:494)

Maybe the fallback for fxLayoutGap is missing?

What is the expected behavior?

this works fine in version 7.0.0-beta19

What is the current behavior?

this is broken in version 7.0.0-beta23

What are the steps to reproduce?

https://stackblitz.com/edit/angular-flex-layout-seed-c3txci

@CaerusKaru
Copy link
Member

This should be a really simple patch, and we'll try to get to it this week, but it might get bumped to next week. Regardless, this will be fixed in the next release.

@smile2014
Copy link

smile2014 commented Feb 25, 2019

fxLayoutGap is set, fxFlex percentage does not take effect
https://stackblitz.com/edit/angular-flex-layout-seed-febhfn
@CaerusKaru

@CaerusKaru CaerusKaru added bug in-progress P1 Urgent issue that should be resolved before the next re-lease and removed needs: investigation labels Mar 16, 2019
@CaerusKaru
Copy link
Member

I'm not seeing the endsWith issue in the latest branch, it's probably been fixed by a previous commit. I am still seeing the lack of removal (see #1037), so I'm tracking that now.

@CaerusKaru
Copy link
Member

Ok, here's the root cause analysis, and it's not pretty (for us, anyway). On each style generation cycle (init, breakpoint activation, etc), we run an analysis on all breakpoints and values to determine which should be activated, or more importantly deactivated. We do this by caching the most recently applied style values that either the style builder or style builder cache have returned. We then remove those styles when we need to.

In the case of layout-gap, however, the primary style generation actually happens as a side effect in the style builder. This is because there are no styles applied to the parent during generation, and thus caching would not apply those styles to the parent (e.g. we need to run the generation every time, no matter what). That's where the problem comes in. Because we don't cache those styles during primary style generation, we can't remove them later when there is no fallback.

We'll try to develop a solution to this, but thought it might be interesting to some to know what's really going on, and why this is taking so long.

@CaerusKaru CaerusKaru added has pr A PR has been created to address this issue and removed in-progress labels Mar 17, 2019
CaerusKaru added a commit that referenced this issue Mar 17, 2019
In most directives, style generation and removal happens in
a contained cycle: styles are generated, cached, and then
removed when no longer needed. This is facilitated by caching
the results of the `StyleBuilder`s for each directive in a
local object on the directive, and then removing those styles
when called.

In the case of `fxLayoutGap`, most of the style generation is
applied to the children, and so in most cases just removing the
parent styles is insufficient, and in other cases, just
ineffective (because no parent styles are actually generated).
To fix this, we override the default style clearing method,
and replace it with one that actually removes the child styles
(and the parent styles if any are present).

Fixes #1011
@moi-j
Copy link

moi-j commented Jul 1, 2019

Error still happening in version 8.0.0-beta.25

@rovercoder
Copy link

rovercoder commented Jul 1, 2019

Ended up overriding MediaMarshaller.prototype.updateElement

const MediaMarshaller_updateElement = MediaMarshaller.prototype.updateElement;

MediaMarshaller.prototype.updateElement = function(element: HTMLElement, key: string, value: any) {
  if (key === 'layout-gap' && (value === null || value === undefined)) {
    value = '0px';
  }
  MediaMarshaller_updateElement.apply(this, [element, key, value]);
};

@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
bug has pr A PR has been created to address this issue P1 Urgent issue that should be resolved before the next re-lease
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants