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

A feature, (open-start) and (close-start), for sidenav dissapeared in beta .10... Please return it #6924

Closed
xtianus79 opened this issue Sep 7, 2017 · 17 comments · Fixed by #7747
Assignees
Labels
P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent

Comments

@xtianus79
Copy link

Bug, feature request, or proposal:

I use this api feature for a very specific case... and there was no deprecation, feature parity, or other info explaining the dissapearance of said feature.

The only thing I read was mdsidenav was changing to mddrawer

What is the expected behavior?

I use the said features to do specific actions when the side panel / drawer is coming onto the screen.

For example, when the drawer first fires I need to fire a function... not when the drawer is finished sliding. The visual affect is greatly diminished without this capability

What is the current behavior?

now, I only have access to open and close via when the side panel is finished coming into it's state.

What are the steps to reproduce?

You'd have to use the api, but alas the optiions are gone.
Providing a Plunker (or similar) is the best way to get the team to see your issue.
Plunker template: https://goo.gl/DlHd6U

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

This was awesome granular control that allowed me to have something disappear off the screen when the panel / drawer began to open.

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

material verison 2 beta .10

Is there anything else we should know?

nope

@crisbeto
Copy link
Member

crisbeto commented Sep 7, 2017

Previously those observables were mainly used to coordinate the animation internally, but that is no longer necessary. You can achieve the same by invoking your callback at the same time that you open/close the sidenav. E.g. what used to be close-start is the equivalent of doing:

closeSidenav() {
  doSomethingOnCloseStart();
  sidenav.close();
}

@xtianus79
Copy link
Author

xtianus79 commented Sep 7, 2017

@crisbeto how would that work for toggle? I am sure it could work to what you're saying but isn't that defeating the ease of using sidenav.toggle?

also, this is not a deprecating change but a breaking change no?

and after thinking about your suggestion is... it just makes it mehhhh considering i have a sperate function for when it opens and when it closes.

@phixMe
Copy link

phixMe commented Sep 8, 2017

The MdSidenav documentation seems to indicate that open-start and close-start are still available. I'm using them to time some parallel animations as the documentation says.

Looking back in the commit history it seems that the eventEmitters were removed by this commit.
(a882546#diff-1bbe0e0324a7c37430acb68a9d1f61ee)

@crisbeto
Copy link
Member

crisbeto commented Sep 8, 2017

The same holds for toggle @xtianus79. If you look at the source, open and close correspond to toggle(true) and toggle(false). The example from above would look like this:

toggleSidenav() {
  if (sidenav.opened) {
    doSomethingOnCloseStart();
  } else {
    doSomethingOnOpenStart();
  }

  sidenav.toggle();
}

@dushkostanoeski
Copy link

How do you fetch the sidenav instance in ts?
I have this happening to me...

@xtianus79
Copy link
Author

@crisbeto I appreciate your answer and for now will implement your suggestion. @crisbeto are you with Angular/Google but because I still feel this should not have been removed in that manner. and is this a final decision. I want to go with what Angular decision is.

@dereekb
Copy link
Contributor

dereekb commented Sep 8, 2017

@xtianus79 I can still access the _animationStarted EventEmitter for MdSidenav. It's not marked private.

export class MyDirective implements AfterContentInit {

    @ViewChild(MdSidenav)
    private _sidenav: MdSidenav;

    ngAfterContentInit() {
        this._sidenav._animationStarted.subscribe(() => {
            console.log('Animation started!');
        });
    }

}

If you need the Outputs, then just make a new directive that provides them.

@dushkostanoeski
I think the issue might be with your view child declaration in the link you posted.

    @ViewChild('rightMenu')
    rightMenu: MdSidenav;

Instead of using the 'rightMenu', try putting in MdSidenav instead so it looks for that class to inject instead of an element.

    @ViewChild(MdSidenav)
    rightMenu: MdSidenav;

@dushkostanoeski
Copy link

I figured out my problem last night and posted what I did in the other issue.

@xtianus79
Copy link
Author

@dereekb how does this help with the need for open and close? Again, the utility of this requires wanting to do someting upon opening the sidenav and upon closing the sidenav with specific methods for each occurance.

@dereekb
Copy link
Contributor

dereekb commented Sep 12, 2017

I'm still a bit confused, but from what I understand it is still possible to gather the same information without the onOpenStart, onCloseStart event emitters (that you're requesting back, right?), as they've just been wrapped into a single _animationStarted.

Couldn't you subscribe to _animationStarted, and read the isOpen/closed state to achieve your original motivation?:

This was awesome granular control that allowed me to have something disappear off the screen when the panel / drawer began to open.

Could you provide a Plunker so I can get a better idea of what you're trying to do?

@xtianus79
Copy link
Author

xtianus79 commented Sep 12, 2017

@dereekb in my use case... I have things that disappear when slidenav starts to open and reappear when the slidenav is closed... has to do with my top level navigation elements.

placing the animationStarted in isOpen / closed state is overengineering for something that handled this preiovusly with no issue. Agian, why remove something and not replace with well defined and clear replacement funtionality?

Weighing the responses here versus questioning why the methods were removed from the API leads me to believe they were removed perhaps prematruely.

Again, I would like to get official word on this before proceeding forward.

@rosslavery
Copy link

rosslavery commented Sep 27, 2017

@crisbeto This has bitten me as well. Despite writing what should be the functional equivalent, the timing of the calls is not the same as before.

My use case is that I have a sidenav with a search input at the top. The open-start event was used to create listeners on that input, and focus the input early enough that if the user types in the field while the open animation is happening, it will register their keypresses.

With the new version, the input is not focussable until the sidenav animation completes, which causes any keydown events during the animation (before the input can be focussed) to be lost.

@rosslavery
Copy link

Seems like my issue may be related to #6376 . Possible that certain actions are being prevented while animating?

Either way, these events were removed without notice and there's at least a few of us that had a legitimate reason for needing them. They provide enough sugar (imo) to justify their existence, even if they can be replaced with a manual implementation.

@josephperrott josephperrott added the P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent label Sep 27, 2017
@crysislinux
Copy link

sidenav.opened property seems become false when the closing animation starts. That makes it inconstant with onClose because onClose fires when the animation ends.

/**
   * Toggle this drawer.
   * @param isOpen Whether the drawer should be open.
   */
  toggle(isOpen: boolean = !this.opened): Promise<MatDrawerToggleResult> {
    this._opened = isOpen;

    if (isOpen) {
      this._animationState = this._enableAnimations ? 'open' : 'open-instant';
    } else {
      this._animationState = 'void';
    }

    if (this._focusTrap) {
      this._focusTrap.enabled = this._isFocusTrapEnabled;
    }

    // TODO(crisbeto): This promise is here for backwards-compatibility.
    // It should be removed next time we do breaking changes in the drawer.
    return new Promise(resolve => {
      first.call(isOpen ? this.onOpen : this.onClose).subscribe(resolve);
    });
  }

@crysislinux
Copy link

the solution from @crisbeto works only for manually toggle the drawer, but when use esc or click on the overlay to close the sidenav, it won't get triggered.

crisbeto added a commit to crisbeto/material2 that referenced this issue Oct 12, 2017
* Re-adds the `openedStart` and `closedStart` events due to popular demand.
* Fixes wrong signature for the returned promise from toggle. The signature states that the resolved value is supposed to be a `MatDrawerToggleResult` while in practice it was `undefined`.
* Splits out a long unit test into two smaller ones.

Fixes angular#6924.
@crisbeto
Copy link
Member

The outputs will be re-added in #7747.

crisbeto added a commit to crisbeto/material2 that referenced this issue Oct 12, 2017
* Re-adds the `openedStart` and `closedStart` events due to popular demand.
* Fixes wrong signature for the returned promise from toggle. The signature states that the resolved value is supposed to be a `MatDrawerToggleResult` while in practice it was `undefined`.
* Splits out a long unit test into two smaller ones.

Fixes angular#6924.
crisbeto added a commit to crisbeto/material2 that referenced this issue Oct 29, 2017
* Re-adds the `openedStart` and `closedStart` events due to popular demand.
* Fixes wrong signature for the returned promise from toggle. The signature states that the resolved value is supposed to be a `MatDrawerToggleResult` while in practice it was `undefined`.
* Splits out a long unit test into two smaller ones.

Fixes angular#6924.
josephperrott pushed a commit that referenced this issue Nov 7, 2017
* Re-adds the `openedStart` and `closedStart` events due to popular demand.
* Fixes wrong signature for the returned promise from toggle. The signature states that the resolved value is supposed to be a `MatDrawerToggleResult` while in practice it was `undefined`.
* Splits out a long unit test into two smaller ones.

Fixes #6924.
@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 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants