Skip to content
This repository has been archived by the owner on Nov 3, 2021. It is now read-only.

Bug 954712 - [Messages] Hide subject field when input is empty and user taps backspace #15118

Closed
wants to merge 1 commit into from

Conversation

rwaldron
Copy link
Contributor

@rwaldron rwaldron commented Jan 8, 2014

subject.dispatchEvent(new CustomEvent('blur'));
subject.dispatchEvent(new CustomEvent('focus'));
assert.isFalse(banner.classList.contains('hide'));
suite('Visibility', function() {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New tests from here to the end

event.preventDefault();

Compose.toggleSubject();
ThreadUI.updateSubjectHeight();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we use here this.updateSubjectHeight?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated

@julienw
Copy link
Contributor

julienw commented Jan 15, 2014

Turns out that both keypress and keydown fire continuously when the backspace is held (on device and desktop)

s***. Any better idea ? Do we have a keyup for each keydown even if the user moves the finger out of the screen ?

@rwaldron
Copy link
Contributor Author

@julienw I've fixed this on desktop, but there is definitely a bug in the keyboard app: it fires a keydown, keypress and keyup event even when holding.

@rwaldron
Copy link
Contributor Author

I've pushed the WIP so you can load it on your device. Load it up and then run adb shell logcat | grep "Console.*sms", here's the STR:

(assuming you're at new message with a subject field open)

  1. type some characters
  2. hold down backspace
  3. observe that UP, DOWN, PRESS are logged over and over

@julienw
Copy link
Contributor

julienw commented Jan 16, 2014

sorry, I couldn't have a look today, keeping this for tomorrow morning!

backspace = function(id) {
ThreadUI.onSubjectKeydown(event);
ThreadUI.onSubjectKeyup(event);
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: you can create event and backspace out of the setup

@julienw
Copy link
Contributor

julienw commented Jan 17, 2014

This needs a rebase

@julienw
Copy link
Contributor

julienw commented Jan 17, 2014

this needs jshint/gjslint lint fixes too :)

@rwaldron
Copy link
Contributor Author

Right, this wasn't "ready" for review yet either—which is why it's filled with console.log, I put those there so you could see the issue I was running into. :P

@julienw
Copy link
Contributor

julienw commented Jan 20, 2014

Yeah but I wanted to r+ it already ;) (and it is, so you can actually merge whenever you want!)

@rwxes
Copy link

rwxes commented Feb 19, 2014

Hi sorry for posting off topic, wrong section, wrong site etc.. I'm mucking around with ffos(1.2) simulator and having trouble with keypress firing with the return key, works in ff on mac as expected.

$(".null").keypress(function(e) {
    var inputText = $(this).val().trim();
    if(e.which == 13 && inputText) {
      var chunks = inputText.match(/.{1,1024}/g)
        , len = chunks.length;
      for(var i = 0; i<len; i++) {
        socket.emit('null', {
          msg: chunks[i]
        });
      }
      $(this).val('');
      return false;
    }
  });

This should work shouldn't it?
Edit: anyone able to tell me where I should be really posting this question?

@rwaldron
Copy link
Contributor Author

@rwxes I think you're better off asking this on a Q&A site: http://stackoverflow.com/questions/tagged/jquery

@rwxes
Copy link

rwxes commented Feb 20, 2014

@rwaldron thanks.

@mozilla-autolander-deprecated
Copy link
Contributor

This pull request has been closed due to tree stability issues. Please rebase and re-open the pull request if you still need to land this. Ensure the gaia-try run is green before landing. Sorry for any inconvenience.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
5 participants