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

Snackbar not closing #1398

Closed
pndewit opened this issue Oct 9, 2017 · 23 comments · Fixed by #4166
Closed

Snackbar not closing #1398

pndewit opened this issue Oct 9, 2017 · 23 comments · Fixed by #4166

Comments

@pndewit
Copy link
Contributor

pndewit commented Oct 9, 2017

What MDC-Web Version are you using?

0.22.0

What browser(s) is this bug affecting?

Chrome (Version 61.0.3163.100 (Official Build) (64-bit))
User agent: "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/61.0.3163.100 Safari/537.36"

What OS are you using?

macOS Sierra (version 10.12.6)

What are the steps to reproduce the bug?

  1. Open Snackbar demo page: https://material-components-web.appspot.com/snackbar.html
  2. Snackbar settings should be left as is (only Dismiss on action enabled)
  3. Click the "Show" button twice and switch browser tab (within the 2750ms timer which should close the first snackbar and open the second one)
  4. Switch back to the snackbar demo tab and the snackbar never closes (even when setting focus on other elements)

What is the expected behavior?

The snackbar should close.

What is the actual behavior?

It doesn't close.

Any other information you believe would be useful?

I was digging through the Snackbar component and found out that it gets focus when switching tab to the tab that is showing the snackbar. When it has focus, it doesn't close (expected behaviour). The blur handler should then kick in when moving focus to some other element, but AFAIK the blur handler doesn't trigger any more. I am not sure yet why. The blur handler does kick in when performing the steps above, but clicking the "Show" button only once at step 3.

Besides the actual bug I'd also like to know why the Snackbar gets focus when switching tab? Shouldn't it just continue (or restart) the snackbar timer for closure?

EDIT:
Found a dirty workaround: manually set the snackbarHasFocus variable to false when the visibility change handler is triggered (make sure the MDC visibility change handler is run first by using setTimeout as it sets the snackbarHasFocus variable to true...) and there is a Snackbar queue.

@pndewit
Copy link
Contributor Author

pndewit commented Oct 9, 2017

I got some more related issues:

  1. Set no Action Text (just empty the field)
  2. Open the snackbar once (click the "Show" button once)
  3. Switch browser tab and switch back to the Snackbar demo tab
  4. Trigger blur (select some text on the background)
  5. Notice that the Snackbar is never removed

This is probably because the focus is set on the action button inside the Snackbar and as there is no action button, the focus can not be set and thus the blur is never triggered. Setting a space character in the Action Text textfield is a workaround for this issue.

@Garbee
Copy link
Contributor

Garbee commented Oct 9, 2017

Besides the actual bug I'd also like to know why the Snackbar gets focus when switching tab?

I'd assume this is when the tab is switched back to the MCW tab being active. The snackbar is given focus since it is used as a priority message system that users may need to interact with within the given time constraint. So making that the focus before returning to page focus is there for screen readers and other accessible technology. This way, users actually get the information provided.

@pndewit
Copy link
Contributor Author

pndewit commented Oct 9, 2017

@Garbee Thank you for your reply and I get your point. But wouldn't it be better to just reset the close timer as the Snackbar does not get removed when having focus (obviously you don't want it to be removed while the user is actively using it)?
This would set it "back" to the original state (when it was opened in the first place, before switching tabs).

@Garbee
Copy link
Contributor

Garbee commented Oct 9, 2017

But wouldn't it be better to just reset the close timer as the Snackbar does not get removed when having focus

Not necessarily. It all depends on tab order within the page. By forcing focus, you're negating any travel time to give the user the most pertinent information as quickly as possible before it leaves.

@pndewit
Copy link
Contributor Author

pndewit commented Oct 9, 2017

Thanks, that answers my question.
Do you (by any chance) know why the blur handler is not triggered when moving focus to some other element after switching tabs and having a queue?

@Garbee
Copy link
Contributor

Garbee commented Oct 9, 2017

I don't know off the top myself. I'd need to do deeper debugging of the code as it runs to understand that. I'm seeing a blur event happen when tabs are switched, so that should trigger the blurring even though it really shouldn't. I think there needs to be some analysis of the blur event to see what the cause is (or simply if the page is visible using the page visibility API) to handle things well for that scenario.

Currently, a blur is a blur is a blur but that isn't necessarily the case given the context and needs of a snackbar.

@rickdesantis
Copy link

rickdesantis commented Oct 19, 2017

I'm coping with this problem checking on tab/window change if the snackbar is still active, disabling and recreating it if so (and after 1.5s for a smooth slide down animation and to give the time to read it).

This is my code, it seems to work on most browsers (I'm using webpack/babili):

document.addEventListener('DOMContentLoaded', function () {
  let first = true
  let hidden = 'hidden'

  // Standards:
  if (hidden in document) {
    document.addEventListener('visibilitychange', onchange)
  } else if ((hidden = 'mozHidden') in document) {
    document.addEventListener('mozvisibilitychange', onchange)
  } else if ((hidden = 'webkitHidden') in document) {
    document.addEventListener('webkitvisibilitychange', onchange)
  } else if ((hidden = 'msHidden') in document) {
    document.addEventListener('msvisibilitychange', onchange)
  } else if ('onfocusin' in document) { // IE 9 and lower:
    document.onfocusin = document.onfocusout = onchange
  } else { // All others:
    window.onpageshow = window.onpagehide = window.onfocus = window.onblur = onchange
  }
  function onchange (evt) {
    let v = 'visible'
    let h = 'hidden'
    let evtMap = {
      focus: v, focusin: v, pageshow: v, blur: h, focusout: h, pagehide: h
    }

    evt = evt || window.event
    let visibility = v
    if (evt.type in evtMap) {
      visibility = evtMap[evt.type]
    } else {
      visibility = this[hidden] ? h : v
    }
    if (first) {
      first = false
      return
    }
    if (visibility === v) {
      let el = document.querySelector('.mdc-snackbar--active')
      if (el) {
        setTimeout(function () {
          el.setAttribute('aria-hidden', true)
          el.classList.remove('mdc-snackbar--active')
          window.snackbar = mdc.snackbar.MDCSnackbar.attachTo(el)
        }, 1500)
      }
    }
  }

  // set the initial state (but only if browser supports the Page Visibility API)
  if (document[hidden] !== undefined) {
    onchange({type: document[hidden] ? 'blur' : 'focus'})
  }
})

@ronnieroyston
Copy link

ronnieroyston commented Nov 8, 2017

[UPDATE] This workaround/fix works 80% of the time. Still having issues.

It seems like using .promise.all would be the proper method for the toast/snackbar. Just .push into an array that get's .all'ed so that multiple toasts can queue up if necessary. Or, I'm sure the jQuery UI sauce has the code that one could copy/paste from.

Workaround/Fix: setTimeout.

    // toastD hack. Toast was sticking when fired from catch block or promise result. This timer fixed it.
    var toastD = function(msg) {
        setTimeout(function() {
            msg = String(msg);
            snackbar.show({
                message: msg,
                timeout: 2750
            });
        }, 500);
    }; 

One last thing. 2+2 is 4, minus 1 is 3; quick maths. smoke trees.🌲

@ronnieroyston
Copy link

I'm not even understanding my visibility is used for this. It's super easy basic position:fixed and just CSS transition it from -96px to whatever (for example). CSS fixed will basically force it off screen. No need to over-engineer it. That's just my 2 cents.

@piotr-cz
Copy link
Contributor

piotr-cz commented Feb 2, 2018

For me snackbar doesn't close even without switching tabs, probably when called from inside a promise.
Workaround mentioned @rhroyston did the job, timeout can be as low as 10ms

@ronnieroyston
Copy link

BTW: My final final solution was to have the action button show in the toast just in case it got stuck, user could close it. This seems to be the best solution.

    <div class="mdc-snackbar" aria-live="assertive" aria-atomic="true" aria-hidden="true">
        <div class="mdc-snackbar__text"></div>
        <div class="mdc-snackbar__action-wrapper">
            <button type="button" class="mdc-snackbar__action-button" style="color:#ffd600"></button>
        </div>
    </div>

    if(doc.querySelector('.mdc-snackbar')){
        var snackbar = new mdc.snackbar.MDCSnackbar(doc.querySelector('.mdc-snackbar'));
    }

    function toast(msg) {
        setTimeout(function() {
            msg = String(msg);
            
            snackbar.show({
                message: msg,
                actionText: 'Close',
                actionHandler: function () {
                }
            });
        }, 500);
    };

@piotr-cz
Copy link
Contributor

piotr-cz commented Feb 4, 2018

Update:
Snackbar doesn't close when shown immediately before or after executing function of History API (either
history.pushState or history.replaceState).

I'm not sure how this is related, but removing focus at this line or
commenting out this line helps:

['touchstart', 'mousedown'/*, 'focus'*/].forEach((evtType) => {
  this.adapter_.deregisterCapturedInteractionHandler(evtType, this.interactionHandler_);
});

On the login screen after user successfully logged in, I'm showing a message Successfully signed in and using history api to redirect to home page. This is when snackbar message gets stuck.
When I don't do a redirect, snackbar closes.

@piotr-cz
Copy link
Contributor

piotr-cz commented Feb 4, 2018

Update:
So the problem is that after login/ logout I'm closing modal window (previously focused element receives a focus element) and showing a snackbar.

Unfortunately this also triggers snackbars' hijackFocus trick and that's why it doesn't close itself and workaround with triggering snacbar with a timeout works (it delays adding focus listener)

This has behavior has been introduced in #852

Snackbar should check if the focused element is a snackbar itself or it's action element and only then use the hijackFocus thingy:

// index.js
return new MDCSnackbarFoundation({
  // ...
  isFocused: () => document.activeElement === getActionButton(),
})

// foundation.js
this.interactionHandler_ = (evt) => {
  if (!this.adapter_.isFocused()) {
    return;
  }
  // ...
}

@piotr-cz
Copy link
Contributor

piotr-cz commented Feb 5, 2018

My use case relates to #1918.
I'm not sure if suggested fix would solve issue described here as it relates to toggling window visibility

@iamenrique
Copy link

Using the Page Visibility API and the Foundation.cleanup_ function worked for me. My code display up to 5 Snackbar at the time and they were not being dismissed after sending the current tab to background.

let hidden, visibilityChange;
if (typeof document.hidden !== "undefined") { // Opera 12.10 and Firefox 18 and later support 
    hidden = "hidden";
    visibilityChange = "visibilitychange";
} else if (typeof document.msHidden !== "undefined") {
    hidden = "msHidden";
    visibilityChange = "msvisibilitychange";
} else if (typeof document.webkitHidden !== "undefined") {
    hidden = "webkitHidden";
    visibilityChange = "webkitvisibilitychange";
}

let _timeoutId;
let handleVisibilityChange = () => {
    if (document[hidden]) {
        // Tab sent to background
        if (typeof _timeoutId !== 'undefined') {
            clearTimeout(_timeoutId)
        }
    } else {
        // Tab came back
        _timeoutId = setTimeout(() => { this.mdcObject.getDefaultFoundation().cleanup_() }, 1000 * (this.stackPosition + 1))
    }
}
document.addEventListener(visibilityChange, handleVisibilityChange, false)

It dismisses the Snackbars one per second depending on my stackPosition value (probably meaningless for you). Furthermore, it handles the scenario where the tab is sent again to background by clearing the current timeout and setting a new one when it comes back.

@deprecatednw
Copy link

Is there any activity on this issue?

@pndewit
Copy link
Contributor Author

pndewit commented Mar 31, 2018

@apoterenko I didn't have the time to get to the bottom of the issue and make a PR with a fix and I was even thinking about implementing a default dismiss action in the snackbar so that users can close them whenever they want. If you (or anyone else) can take a look and see if he can find the real issue and maybe create a PR?

@elitegoliath
Copy link

Still watching this issue. A manual dismiss action would be quite lovely as a bandaid until a true fix can be made.

@pndewit
Copy link
Contributor Author

pndewit commented May 24, 2018

@elitegoliath Not sure if you are asking for a manual dismiss action, but I am pretty sure this won't be implemented in MDC. You could implement this in your own project as long as the problem exists though, something like:

<div class="mdc-snackbar" aria-live="assertive" aria-atomic="true" aria-hidden="true">
  <div class="mdc-snackbar__text"></div>
  <div class="mdc-snackbar__action-wrapper">
    <button type="button" class="mdc-snackbar__action-button"></button>
  </div>
</div>
import {MDCSnackbar} from '@material/snackbar';

const snackbarElement = document.querySelector('.mdc-snackbar');
const snackbar = new MDCSnackbar(snackbarElement);
const dataObj = {
  message: 'Message that is possibly not automatically hidden',
  actionText: 'Dismiss',
  actionHandler: function () {
    snackbarElement.setAttribute('aria-hidden', true);
    snackbarElement.classList.remove('mdc-snackbar--active');
  }
};

snackbar.show(dataObj);

@piotr-cz
Copy link
Contributor

Guys, see if #2183 fixes issue at least for some of you

@pndewit
Copy link
Contributor Author

pndewit commented Jul 12, 2018

Just found another way of making the snackbar hard to close and its easy to reproduce.

  1. Open the MDC snackbar demos: https://material-components-web.appspot.com/snackbar.html
  2. Press the "Show" button
  3. Press the "Undo" button in the snackbar before it closes
  4. Wait for at least 3 seconds (the snackbar timeout)
  5. Press the "Show" button again
  6. Note that the snackbar opens WITH the Undo action button, but the button is removed shortly after and the snackbar is not closing any more... Making it really hard to close...

@ronnieroyston
Copy link

ronnieroyston commented Jul 12, 2018 via email

@alphabt
Copy link

alphabt commented Aug 7, 2018

I'm also having the same issue. My scenario is after logging out I redirect to the login page and the snackbar shows a "successfully signed out" message but it gets stuck.

My guess is that my login's username field has an autofocus attribute, and upon loading the page it most likely stole the focus away from the snackbar causing it to get stuck. If I remove autofocus the snackbar closes normally.

acdvorak added a commit that referenced this issue Dec 13, 2018
### Issues fixed

* Fixes #4005 (via new `mdc-snackbar-viewport-margin()` mixin)
* Fixes #3981
* Fixes #2916 (via new `MDCSnackbar:closing` and `MDCSnackbar:closed` events)
* Fixes #2628
* Fixes #1466 (via new `close()` method)
* Fixes #1398
* Fixes #1258
* Fixes #11 (the label now expands indefinitely to fit the text)
* Refs #2813

### Screenshots

![Baseline without action button](https://user-images.githubusercontent.com/409245/49956261-a080ca80-feb9-11e8-8287-b7e805344115.png)

![Baseline with action button](https://user-images.githubusercontent.com/409245/49956109-4a138c00-feb9-11e8-9213-3241fa86a1b8.png)

![Stacked layout](https://user-images.githubusercontent.com/409245/49956173-6a434b00-feb9-11e8-8a03-2e58ccc8f763.png)

BREAKING CHANGE: Snackbar's DOM and APIs have changed to match the latest design guidelines. See the Snackbar documentation for more information.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants