backspace on chips trigger browser "back"

[Thanood]

Description

When trying to delete a chip with backspace (twice, the first highlights the chip, the second removes the chip), the chip is removed but when you visited another page before, the browser back history function is triggered and thus navigates to the previous page.

(on Chrome 54)

Repro Steps

For example, open the Materialize homepage, then go to "Components => Chips", browse to the "Plugin" section there. Focus the chip input of the example and delete the last chip by hitting backspace twice.

You should be on the homepage again. 😃

Screenshots / Codepen

See Materialize homepage or: http://codepen.io/anon/pen/wogVEY?editors=1010
(you would need to have a previous page when using the CodePen)

[developeranirudhprabhu]

@Thanood I was not able to reproduce on your codepen or the official website. Can you check again and confirm on the latest build?

[Thanood]

@developeranirudhprabhu It still happens on the official web page and in the CodePen (if there's a previous page of course). I've tested with Chrome, Edge and Firefox.

Which browser are you using?

[developeranirudhprabhu]

@Thanood I'm testing on chrome 55. I followed the steps you have mentioned but could not reproduce the issue

[Thanood]

Interesting.. I had an older Chrome version and upgraded to 55. Doesn't happen anymore in Chrome 55.
But still happens on Edge and FF (probably IE as well). 😃

Would a preventDefault() do the trick here like there is one for the enter event? I could issue a PR for that, too. 😃

[developeranirudhprabhu]

I need to check why it happens. Ideally if the chips.length is 0 then nothing should happen

[Thanood]

Just to be precise, please note that this does not only happen when chips.length == 0.
The code line I posted would be the wrong place, though. 😃

[developeranirudhprabhu]

So you mean to say that irrespective of number of chips double backspace causes issues?

[Thanood]

Yes.
The default action for the backspace key in the majority of browsers (including Chrome up to a certain point) is history.back().

In fact if you hit backspace on Chrome twice the browser shows a popup indicating that this binding is now alt + backspace.

Thus my proposal to prevent the default action if e.keyCode === 8. Just like it is done with the enter key (without adding a new chip, of course).

[developeranirudhprabhu]

I think some thing is going wrong here: https://github.com/Dogfalo/materialize/blob/master/js/chips.js#L84

[developeranirudhprabhu]

Can you try removing

  if ($(e.target).is('input, textarea')) {
          return;
        }

And see if it helps

[developeranirudhprabhu]

Update: Removing previously mentioned line solves the problem. Would create the PR for same tonight

[Thanood]

Interesting.. I'll try it, too when I have some time (I'm currently at work).
Thanks! 👍

[developeranirudhprabhu]

Sure. Please try and let me know your feedback

[developeranirudhprabhu]

@Thanood did you try? Did it work for you?

[Thanood]

@developeranirudhprabhu Not quite.. Hard to describe somehow.

With these lines not removed, the chip to be deleted is first selected (changes its background color to accent) then after another backspace it is removed and that is when the browser navigates back.

Now with these lines removed chips behave differently. The chip to be deleted is not selected and is removed without triggering history.back(). Instead, the next chip is selected and when you then hit backspace it is removed from the list and still triggers history.back().. 😕

So when you have these chips:
Apple Microsoft Google

Focus the chip editor and press backspace it looks like this:
Apple _Microsoft_
(note that I've put _ around the chip being selected)

If you nowpress backspace again it triggers history.back().
I guess the event target is not the input anymore but the chip's <div>.

Btw. if you blur the input before the second backspace and focus it again it works. But tbh. I think the behaviour of first selecting the chip is a bit nicer. 😃

I've tried to debug this behaviour. I think history.back() even happens before any event is caught by the chips implementation. With that I mean the second backspace after the chip is being displayed as selected.
Of course that also means my "proposal" of using e.preventDefault() doesn't work either. I'll try some more if I find the time today.

[developeranirudhprabhu]

ok i will think of some alternate solution. I think we need to stop the event propagation in that case

[developeranirudhprabhu]

@Thanood After looking through the issue. I think opting "preventDefault()" technique works better. I added following line

if(e.which == 8){e.preventDefault()};

on line above following

if ($(e.target).is('input, textarea')) {
          return;
        }

It doesnt seem to trigger history.back. I tested it only on firefox. Here i think what was needed is the check of back space at the very beginning.
Let me know your thoughts

[developeranirudhprabhu]

@Thanood Lemme know if it works for you. Also if you plan to create PR?

[Thanood]

@developeranirudhprabhu That seems to fix it! 😃
I've tried on IE11 and an older Chrome version (also Vivaldi which is basically the same as an older Chrome version).

I can create a PR but I need some time to do that. I'll try tomorrow.
If you can do it faster then please feel free to go ahead. 😃

Thanks!

[developeranirudhprabhu]

@Thanood I will do it today

[Thanood]

Great, thanks! 👍

[developeranirudhprabhu]

@Dogfalo @acburst Please merge the PR and close the ticket

[Dogfalo]

Fixed in dbf25fa

[developeranirudhprabhu]

@Thanood Can you check the latest fix and confirm?