Visible property for toggle split area on and off, which changing structure of the view #8
Conversation
The purpose of visibility for split areas if to establish persiting router outlets within each split area. Since new router v3. Router outlet within conditional split areas can't be consistently relied on. Extensive testing were done, to make sure this change doesn't impact how the split areas' conditional rendering, just add ability to toggle areas on/off visually.
|
Nice, Do you try to mix 'conditionnal areas' and 'visibibity areas' and toggling them? |
|
I didn't want take conditional areas out, fear that will break existing
implementations.
Visible areas just sit side by side with conditional areas
…On Fri, Jan 20, 2017, 12:59 AM bertrandg ***@***.***> wrote:
Nice,
Yes I understand the need, I will take time to test it and then, if
everything's fine, merge it and add an example into website.
Do you try to mix 'conditionnal areas' and 'visibibity areas' and toggling
them?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#8 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/APGpoZ9MklN-m4eiW2cYXiVyegqTtd-Eks5rUHd7gaJpZM4Lot6w>
.
|
of area, instead of conditional create the area. Also, extracted some hard-coded styles into css classes, that will be contained within the component.
|
hi @jitsmaster What did you mean by: "I didn't want take conditional areas out, fear that will break existing implementations." ? I've seen you add an other feature about animation and changed styles, can you keep it for an other pull request please. |
|
Sorry for the confusion. Just meant I didn't touch the Ngif part at all and
make sure it still works.
I am waiting for you to approve the visibility feature first before I
create the pull request for animations.
I will create the pull request after this one is approved.
Thank you
Arnold
…On Fri, Jan 27, 2017, 3:31 PM bertrandg ***@***.***> wrote:
hi @jitsmaster <https://github.com/jitsmaster>
I've just played with your [visible] feature and it seems to works fine
mixed with *ngIf:
https://plnkr.co/edit/GyQnxE?p=preview
What did you mean by: "I didn't want take conditional areas out, fear that
will break existing implementations." ?
I've seen you add an other feature about animation and changed styles, can
you keep it for an other pull request please.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#8 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/APGpodBpFIh6mKP3cQKM9n1MUqbPzic2ks5rWn5hgaJpZM4Lot6w>
.
|
transitionend events of the split areas, after show/hide animations finishes. Also, upgrade Angular 2 to 2.4.3.
|
Checking on how soon you can merge in my pull request. My app is getting pretty close to production state. Would really get the NPM package to be updated. Also, I have created a new 'layoutEnd' event for split component. It is trigger after the animation ended for split area show/hide. Would love to pull this one into master too. |
|
Yes I understand, I do that before the end of the week! :) |
|
hi @jitsmaster I would like to first merge your 3 commits from 19 January (only about toggling visibility) but there are 2 more about animation/layoutEnd in the pull request.. Is it possible to lock your pull request to specific commit (e539e79) ? About animation/layoutEnd, I haven't test it already but it seems to change a lot of things I need to test because I use it too on a prod app at work. (multi browser animation / multiple "soft removed" at same time) |
|
Let me create a branch just for the first 3 commits and send a new pull
request.
…On Fri, Feb 3, 2017, 4:01 AM bertrandg ***@***.***> wrote:
hi @jitsmaster <https://github.com/jitsmaster>
I'm finally on it! 😳
I would like to first merge your 3 commits from 19 January (only about
toggling visibility) but there are 2 more about animation/layoutEnd in the
pull request..
https://github.com/bertrandg/angular-split/pull/8/commits
Is it possible to lock your pull request to specific commit (e539e79
<e539e79>)
?
About animation/layoutEnd, I haven't test it already but it seems to
change a lot of things I need to test because I use it too on a prod app at
work. (multi browser animation / multiple "soft removed" at same time)
And is there a specific reason to update dependencies to 2.4.3?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#8 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/APGpoSh6kqebquoZGjYirCURdmubxnX1ks5rYxcIgaJpZM4Lot6w>
.
|
|
Done. A new pull request is created, that includes only the visibility feature. Let me know if you want to reject this pull request and let me create another one after visiblity feature is merged. |
simple visibility toggle feature (c7c92b9 + e539e79)I tested it more today and found a bug, check this plunker: Click button "toggle visible C", areaC disapears but there's still a gutter on the right that shouldn't be there: Should be solvable with this but need to be tested: animation/layoutEnd feature (ee14002 + cee8d50)
I think the best way to merge it quickly is to first make a new pull-request with c7c92b9 + e539e79 + bug fix. And sorry for the time it took me to answer you with details. |
|
Thank you for finding that bug. I will put in the fix in the other pull request. Regarding the layoutEnd event firing, it was caused by transitionend event somehow fired multiple time on both chrome and firefox, so I had to debounceTime on it. Also I had to merge all events together in case 2 or more areas are toggled in the same time, and their transitionend firing will not be in the same time. I will test the css changes in shadow dom scenario |
|
Cool, I wait for your commit on the other PR and will merge it quickly. |
|
That is weird you still have to import the specific operators. I haven't needing to do it for quite a while. Angular 2.4.1 had a very strange bug of sometimes AOT build put ngfactory files in the source folder. 2.4.3 seems to fix the issue. |
|
You are right that /deep/ indeed doesn't work on native view encapsulation. What is even worse if the flex direction will not be set with class. I see you are directly setting the style of flex direction on hostbinding. Is it your recommended approach? |
|
Actually, take that back. The only thing that wasn't working was the vertical direction. As soon as I use the same way of hostbinding seting flex direction. Everything works, including animation, in native mode. I am creating a pull request now |
|
Ok, yes |
The need arises due to the need of place router outlets inside split areas that need to be toggled on/off.
Since Router v3, router outlet inside a conditional area will not persist. This make routing on that outlet generating errors.
The purpose of this change is to simply hide the split area, not removing it, this way the router outlet will always be a there, and any other view children that need to persist will be always there too.