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

fxShow/fxHide is setting display: block on inline elements #724

Closed
adamdport opened this issue Apr 26, 2018 · 13 comments · Fixed by #881
Closed

fxShow/fxHide is setting display: block on inline elements #724

adamdport opened this issue Apr 26, 2018 · 13 comments · Fixed by #881
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

@adamdport
Copy link

Bug, feature request, or proposal:

bug

What is the expected behavior?

fxHide shouldn't affect the original display of the element

What is the current behavior?

fxHide is sometimes setting display:block;

What are the steps to reproduce?

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

What is the use-case or motivation for changing an existing behavior?

I sometimes want inline elements that hide at certain breakpoints

Which versions of Angular, Material, OS, TypeScript, browsers are affected?

"@angular/core": "5.2.9",
"@angular/flex-layout": "5.0.0-beta.14",
"@angular/material": "5.2.4",

Is there anything else we should know?

#320 fixed some instances of this behavior, but not all. Perhaps this is related to transclusion?

@CaerusKaru
Copy link
Member

Duplicate of #142?

@jotatoledo
Copy link

jotatoledo commented May 14, 2018

Just noticed this issue, some <span fxHide.lt-md> are now block components, which partially breaks some layouts.

@CaerusKaru
Copy link
Member

Oh I see the problem now. The fallback, ideally, should be blank. I'll try to get in a fix for this by beta 17

@CaerusKaru CaerusKaru self-assigned this Jun 11, 2018
@CaerusKaru CaerusKaru added bug P1 Urgent issue that should be resolved before the next re-lease labels Jun 11, 2018
@CaerusKaru CaerusKaru added this to the v6.0.0-beta.17 milestone Jun 11, 2018
@FrancisS
Copy link

FrancisS commented Sep 7, 2018

Any progress on this? @CaerusKaru @ThomasBurleson

@FrancisS
Copy link

FrancisS commented Oct 1, 2018

If anyone is looking for a temp solution this is what we had to do

  @media (min-width: 1440px) {
    mat-header-cell,mat-cell{
      &[fxHide\.lt-lg]{
        display: flex !important;
      }
    }
  }

It sets the display to flex when the column is not hidden for a given breakpoint.
You will need separate rules for each breakpoint you use.

@CaerusKaru
Copy link
Member

I've looked into this, and unfortunately while I can plainly see this replicated in the StackBlitz, any test cases I create for default inline elements (e.g. <span>) pick up the display: inline; styles correctly. That being said, I'm still looking into this, and my current thinking is that <mat-placeholder> is initially a block element, and then the browser computes its parent and determines it to be inline. I'll update as I continue to investigate this and #848

@adamdport
Copy link
Author

@CaerusKaru here's another reproduction using mat-table. The cells are supposed to be display:flex, but fxHide is setting them to display:block. Hope it helps. https://stackblitz.com/edit/angular-vpw5kv-r4td8o

@aklauza
Copy link

aklauza commented Oct 28, 2018

Found the same issue with a table column that I was trying to hide for small devices (using <td fxHide.xs>). My table is inside a mat-tab, thus, it's initially hidden, but when the tab is activated the fxLayout engine realizes it's not an xs device but a desktop and injects an inline style="display:block". The problem is that tds should be "display:table-cell", not block. My workaround is not use for this case fxHide.xs, but the following:

.hide {
   display: none;
}

and in the html:

<td ngClass.xs="hide">...</td>

@CaerusKaru
Copy link
Member

@aklauza Can you pull together a StackBlitz using your example? The only issue with @adamdport's examples are that they're all using Angular-defined elements, so it's tough to know what the defaults are supposed to be.

@CaerusKaru
Copy link
Member

I think I've got this patched in #881. Please build to make sure. If I don't hear back either way in the next week, I'll push it to the nightly builds (since it's not that far ahead of NPM, it shouldn't impact too many if it breaks something).

@CaerusKaru CaerusKaru added the has pr A PR has been created to address this issue label Nov 10, 2018
@adamdport
Copy link
Author

@CaerusKaru what's the simplest way to test a branch? It looks like the package name is flex-layout-srcs which is throwing me off. Do I need to clone it, change it back and install that or is there a simpler way?

@CaerusKaru
Copy link
Member

What I do is clone the branch, build the lib, and then run npm install <src directory> which would be dist/releases/flex-layout IIRC.

Or you can ask @srcn what they did in #848

CaerusKaru added a commit that referenced this issue Nov 13, 2018
…ayout (#881)

* Fixes `fxFlex` child interfering with parent `fxShow`/`fxHide`
* Fixes `fxShow`/`fxHide` not applying the correct computed styles for custom elements

Closes #848 
Closes #724
@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