Closed Bug 805167 Opened 12 years ago Closed 6 years ago

Reducing ring tone volume to vibration should vibrate shortly

Categories

(Firefox OS Graveyard :: Gaia::System, defect)

Product:
Firefox OS Graveyard
Component:
Gaia::System
Type:
defect
Priority:
Not set
Severity:
trivial

Tracking

(blocking-basecamp:-)

Status:
RESOLVED WONTFIX
Project Flags:
blocking-basecamp -

People

(Reporter: wenzel, Unassigned)

References

Details

(Keywords: b2g-testdriver, unagi)

Attachments

(1 file, 4 obsolete files)

[gaia] joshua-s:bug-805167 > mozilla-b2g:master
46 bytes, text/x-github-pull-request
alive
: review+
Details | Review 
There's no tactile feedback when I reduce the ring tone volume to vibrate vs. reducing it to "off". When reaching the "vibrate" mode, the phone should vibrate very shortly to indicate the status.
blocking-basecamp: ? → -
Component: Gaia → Gaia::System
Assignee: nobody → skethu
Attached file Pull Request (obsolete) — 

        Attachment #8372866 -
        Flags: ui-review?(swilkes)

        Attachment #8372866 -
        Flags: review?(kgrandon)

    
    

    
  
Comment on attachment 8372866 [details] [review]
Pull Request

You probably need a review from alive or tim here. Made a few nits on github though.

        Attachment #8372866 -
        Flags: review?(timdream)

        Attachment #8372866 -
        Flags: review?(kgrandon)

        Attachment #8372866 -
        Flags: review?(alive)

    
    

    
  

      
      
      
      

        Attached file
          
        Pull Request - Fixed braces issue (obsolete)
        — 
      

    


  

        Attachment #8372866 -
        Attachment is obsolete: true

        Attachment #8372866 -
        Flags: ui-review?(swilkes)

        Attachment #8372866 -
        Flags: review?(timdream)

        Attachment #8372866 -
        Flags: review?(alive)

        Attachment #8372886 -
        Flags: ui-review?

        Attachment #8372886 -
        Flags: review?(timdream)

    
  

        Attachment #8372886 -
        Flags: review?(alive)

    
    

    
  
Comment on attachment 8372886 [details]
Pull Request - Fixed braces issue

Alive could review this.

https://github.com/mozilla-b2g/gaia/pull/16104

Please squash two commits in your branch.

        Attachment #8372886 -
        Flags: ui-review?(firefoxos-ux-bugzilla)

        Attachment #8372886 -
        Flags: ui-review?

        Attachment #8372886 -
        Flags: review?(timdream)

    
    

    
  
Comment on attachment 8372886 [details]
Pull Request - Fixed braces issue

Flagging Harly for UI review on the patch here.

        Attachment #8372886 -
        Flags: ui-review?(firefoxos-ux-bugzilla) → ui-review?(hhsu)

    
    

    
  
Comment on attachment 8372886 [details]
Pull Request - Fixed braces issue

Please squash your commit and ask review again

        Attachment #8372886 -
        Flags: review?(alive)

    
    

    
  
Comment on attachment 8372886 [details]
Pull Request - Fixed braces issue

I have used the patch on my Inari device, and it seems to work fine when I lower the volume using volume rocker to vibrate that the phone will give vibration feedback. However, it will not give vibration feedback when I turn on the Vibrate switch in Settings.

        Attachment #8372886 -
        Flags: ui-review?(hhsu) → ui-review-

    
    

    
  
I wasn't aware this bug extended into the settings for vibration as well. I thought it was only for the volume rocker being reduced to zero when vibration is on. I will make that change for settings as well and post a new review.

    
    

    
  

      
      
      
      

        Attached file
          
        Pull Request (obsolete)
        — 
      

    


  
The vibration should work in settings as well as with the volume rocker now.

        Attachment #8372886 -
        Attachment is obsolete: true

    
  

        Attachment #8376041 -
        Flags: ui-review?(swilkes)

        Attachment #8376041 -
        Flags: review?(dmarcos)

    
    

    
  
Sending this review to Alive Kuo. Alive, this is the first contribution of one of the students at Facebook Open Academy!

        Attachment #8376041 -
        Flags: review?(dmarcos) → review?(alive)

    
    

    
  
Comment on attachment 8376041 [details] [review]
Pull Request

See github.

I think it's almost done!

        Attachment #8376041 -
        Flags: review?(alive)

    
    

    
  
Would love to see this landed. Any ETA on an updated patch fixing Alive's comments?
Blocks: fxos-papercuts
Flags: needinfo?(skethu)

    
    

    
  
Sorry about that Dietrich, I have a huge assignment due tonight so I wasn't able to post a new patch. I should post one tomorrow morning!
Flags: needinfo?(skethu)

    
    

    
  

      
      
      
      

        Attached file
          
        Pull Request (obsolete)
        — 
      

    


  

        Attachment #8376041 -
        Attachment is obsolete: true

        Attachment #8376041 -
        Flags: ui-review?(swilkes)

        Attachment #8381922 -
        Flags: review?(autonome+bztest)

        Attachment #8381922 -
        Flags: review?(alive)

    
    

    
  
Comment on attachment 8381922 [details] [review]
Pull Request

Thanks!

        Attachment #8381922 -
        Flags: review?(alive) → review+

    
    

    
  
Any updates on this?

    
    

    
  
(In reply to Joshua Smith [:joshua-s] from comment #16)
> Any updates on this?

The pull request in comment 14 has gone stale and was closed, but afaict not resubmitted. :(

    
    

    
  
(In reply to Fred Wenzel [:wenzel] from comment #17)
> The pull request in comment 14 has gone stale and was closed, but afaict not
> resubmitted. :(

Hmm, this is a nice bit of polish. Skethu: can you update the PR, or I can if you are not available.
Flags: needinfo?(skethu)

    
    

    
  
Hi Joshua I am no longer available to work on this could you pick it up for me?
Flags: needinfo?(skethu)
Assignee: skethu → thewanuki

        Attachment #8381922 -
        Attachment is obsolete: true

        Attachment #8381922 -
        Flags: review?(autonome+bztest)

    
    

    
  
Comment on attachment 8601827 [details] [review]
[gaia] joshua-s:bug-805167 > mozilla-b2g:master

I created an updated PR with Soumya's changes. Please review it and let me know if anything needs to be changed.

        Attachment #8601827 -
        Flags: review?(alive)

    
    

    
  
Comment on attachment 8601827 [details] [review]
[gaia] joshua-s:bug-805167 > mozilla-b2g:master

Thanks!

        Attachment #8601827 -
        Flags: review?(alive) → review+

    
    

    
  
Alive can you land this? I don't have push access.
Flags: needinfo?(alive)

    
    

    
  
(In reply to Joshua Smith [:joshua-s] from comment #23)
> Alive can you land this? I don't have push access.

This is what we have autolander for :) Just add checkin-needed after a peer review and it should run tests and land if green.
Flags: needinfo?(alive)
Keywords: checkin-needed

    
    

    
  
Thanks Kevin! Never used Autolander before :)
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED

    
    

    
  
This causes the device to vibrate during the boot process. We shouldn't do this. Alive can you take care of it?
In somehow related note this also breaks our pre-allocated process (bug 1162812).
Flags: needinfo?(alive)

    
    

    
  
Backing out for now due to request for causing performance issues. Adding a needinfo for myself to take a look at re-landing with a fix if Alive doesn't get to it first.

https://github.com/mozilla-b2g/gaia/commit/33382edebf24666b19594410e94bed26a17d4459
Status: RESOLVED → REOPENED
Flags: needinfo?(kgrandon)
Resolution: FIXED → ---

    
    

    
  
:(

John reminded me we had already vibrated when the user change the volume to 0 but it won't vibrate when the user changes the vibration settings.

Is this what we want for the original bug description? If yes, let's close this bug.
If not, let's fix this bug by a new patch.
Flags: needinfo?(alive) → needinfo?(firefoxos-ux-bugzilla)

    
    

    
  
IMO it should vibrate for both (the vibration in Settings is analogous to the sound clip played on volume change).

I can also create another fix.

    
    

    
  
Josua - it looks like you're working on a patch for this? Also please remember that we shouldn't vibrate on startup. Let me know if you need any help here, thanks!
Flags: needinfo?(kgrandon)

    
    

    
  
NI'ing Jenny for feedback on vibration settings (See comment 29 from Alive). Jenny, Harly has looked into this patch previously if you have any questions. 

- Rob
Flags: needinfo?(firefoxos-ux-bugzilla) → needinfo?(jelee)

    
    

    
  
I think it's an enhancement to have a tactile feedback when user change the vibrate settings in Settings app. Let's do it, thanks!
Flags: needinfo?(jelee)

    
    

    
  
Alright then, I'll put together a patch!

    
    

    
  
To clarify:
- The original bug (to vibrate when user sets volume to 0 via hardware buttons) is fixed in other code?
- We want to vibrate when the sliders for Media, Ringtones and Notifications, or Alarm are set to 0?
- We do not want to do this on startup!

Do I also need to be concerned about the performance issue?

    
    

    
  
(In reply to Joshua Smith [:joshua-s] from comment #35)
> To clarify:
> - The original bug (to vibrate when user sets volume to 0 via hardware
> buttons) is fixed in other code?

To my knowledge it is fixed.

> - We want to vibrate when the sliders for Media, Ringtones and
> Notifications, or Alarm are set to 0?

Slider controls volume and volume only, so it shouldn't trigger vibration. Another reason is that on the panel, it is clear that vibration is controlled by another UI element (which will give corresponding feedback once we land the patch).

Thanks!
Assignee: thewanuki → nobody

    
    

    
  
Firefox OS is not being worked on
Status: REOPENED → RESOLVED
Closed: 9 years ago → 6 years ago
Resolution: --- → WONTFIX

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: