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

fxLayoutGap not allows working on resize #606

Closed
tmburnell opened this issue Feb 9, 2018 · 13 comments · Fixed by #931
Closed

fxLayoutGap not allows working on resize #606

tmburnell opened this issue Feb 9, 2018 · 13 comments · Fixed by #931
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

@tmburnell
Copy link

tmburnell commented Feb 9, 2018

Bug, feature request, or proposal:

If you load the page with an element hidden then resize to where it is visible the gap is not calculated (leaving no space). Where if the page is loaded where it is visible you can resize all you want and the gap displays correctly.

What are the steps to reproduce?

Providing a StackBlitz (or similar) is the best way to get the team to see your issue.

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

So it appears that the gap is only calculated on init of the page, and not on the change event. This might have been done for performance ... but leaves for inconstant ui.
Or it was just overlooked.

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

Angular: 5.2.0
flex-layout: 2.0.0-beta.12

*** Amended ***
I noticed one more thing after trying to make the colors more visible:
when i said if the page loads where the content is visible, the gap stays even after the element hides (which seems like a related bug ... because it should go away).

@CaerusKaru CaerusKaru added bug P2 Issue that is important to resolve as soon as possible labels Feb 10, 2018
@CaerusKaru CaerusKaru added this to the v5.0.0-beta.14 milestone Feb 10, 2018
@CaerusKaru
Copy link
Member

CaerusKaru commented Mar 16, 2018

Ok this is definitely an issue, and it stems from the fact that most directives either watch for internal changes or layout direction changes (e.g. row->column) only.

This will require some careful design to get right, but the issue you identified (in both cases) is real and we're investigating.

@CaerusKaru CaerusKaru self-assigned this Mar 16, 2018
@CaerusKaru CaerusKaru added P1 Urgent issue that should be resolved before the next re-lease and removed P2 Issue that is important to resolve as soon as possible labels Mar 18, 2018
@talamaska
Copy link

Is this some regression? I didn't noticed it didn't work on beta12

@CaerusKaru
Copy link
Member

@talamaska It doesn't look like a regression to me, it's pretty insidious and tough to replicate, but it's definitely an issue.

@CaerusKaru
Copy link
Member

This might be fixed by #900. @tmburnell can I count on you to verify once it gets merged?

@tmburnell
Copy link
Author

I would, but I cant figure out how to build master to test with it. I have always pulled from NPM which is already built. I did try pulling master and building it, however every run command ended up getting this:

(function (exports, require, module, __filename, __dirname) { import {createPackageBuildTasks} from 'lib-build-tools';
                                                                     ^

SyntaxError: Unexpected token {
    at new Script (vm.js:79:7)
    at createScript (vm.js:251:10)
    at Object.runInThisContext (vm.js:303:10)
    at Module._compile (internal/modules/cjs/loader.js:656:28)
    at Module._extensions..js (internal/modules/cjs/loader.js:699:10)
    at Object.require.extensions.(anonymous function) [as .ts] (/Users/#####/dev/flexlayout-issue/node_modules/flex-layout-srcs/node_modules/ts-node/src/index.ts:384:14)

@CaerusKaru
Copy link
Member

@tmburnell Once it gets merged, you'll be able to use the nightly releases. You can install the nightly builds by running npm i angular/flex-layout-builds. But it hasn't been merged yet.

@CaerusKaru
Copy link
Member

@tmburnell Although I don't think it's been fixed, could you test this with 7.0.0-beta.21? For reference to future me: the problem is likely that the childList needs to be watched as well to trigger a refresh. This can be done in two ways: 1) have the MutationObserver watch child attributes as well as additions/removals (could be expensive), or 2) have fxShow and fxHide trigger a reflow of a parent layout-gap if present.

@tmburnell
Copy link
Author

Using this stackblitz: https://stackblitz.com/edit/angular-flex-layout-seed-3xuyme?file=package.json
It is not 100% correct.
It looks like it works when you go from large to small, however if you go from small to large it does not look correct.

@CaerusKaru
Copy link
Member

In fact, it's worse than that. It's completely unpredictable because the sequence of events is not known to either directive, and appears to be different based on each breakpoint activation/deactivation. I've pinpointed a solution, and I'll try to get it in for the next release.

Note to self: the solution is to check the marshal values for the children instead of going right to the styler. This avoids the race condition.

@CaerusKaru
Copy link
Member

This will be patched in #931

@CaerusKaru CaerusKaru added has pr A PR has been created to address this issue and removed in-progress labels Dec 17, 2018
@CaerusKaru
Copy link
Member

@tmburnell Please test this in your app (for some reason I can't get this nightly build to work on StackBlitz). I put the example in our demo-app and it worked as expected.

@tmburnell
Copy link
Author

@CaerusKaru My stackblitz example appears to be working. it will take me some time to remember which application I was working on when i noticed this bug. But my guess is ... if the stackblitz example works then it will work too.

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

3 participants