GNOME Bugzilla – Bug 649586
PopupSliderMenuItem: Enable slider use with a single click-hold-move-release
Last modified: 2013-08-30 15:56:03 UTC
It's quite handy to set the volume with a single click-hold-move-release so use the 'activate' signal to move the handle.
Created attachment 187368 [details] [review] PopupSliderMenuItem: Enable slider use with a single click-hold-move-release
(In reply to comment #0) > It's quite handy to set the volume with a single click-hold-move-release so > use the 'activate' signal to move the handle. What do you mean with "click-hold-move-release"? It already works like that: you can click and drag while holding, or you can just click on the target position. If not there is a bug somewhere. (Also, if gnome-shell master, you can click the whole highlighted area, not just the blue bar)
I assume he means clicking on the Volume icon, moving down to the slider area, and moving the mouse left/right without releasing.
(In reply to comment #3) > I assume he means clicking on the Volume icon, moving down to the slider area, > and moving the mouse left/right without releasing. Yes, and when releasing, the handle moves to where the pointer was at the moment. But this patch is quite incomplete. I still want to make it: a) move the slider while moving the pointer with the button pressed and b) make it do the "blip" sound on activation. I can't look into that today but that's where I'm heading with this.
Created attachment 187969 [details] [review] PopupSliderMenuItem: Enable slider use with a single click-hold-move-release It's quite handy to set the volume with a single click-hold-move-release.
(In reply to comment #4) > But this patch is quite incomplete. I still want to make it: a) move the slider > while moving the pointer with the button pressed and b) make it do the "blip" > sound on activation. OK, I've made these changes now and I think it works quite well. This has a problem though: anyone using PopupSliderMenuItem will get a slider that changes value as long as BUTTON1 is pressed while moving the pointer over it since we don't have a grab. Right now, only the volume menu uses this kind of MenuItem so I don't think this a problem for now, but if you think it's best, I can create a subclass of PopupSliderMenuItem with this behavior in status/volume.js and keep this class as it was.
Created attachment 188131 [details] [review] PopupSliderMenuItem: make slider menu items activatable Keeping the volume menu open after setting the desired volume isn't that useful and forces a second click (or an Esc press) to dismiss it. Allow for the sliders to be used with a single click-hold-move-release.
Review of attachment 188131 [details] [review]: ::: js/ui/popupMenu.js @@ +498,3 @@ this.actor.connect('button-press-event', Lang.bind(this, this._startDragging)); this.actor.connect('scroll-event', Lang.bind(this, this._onScrollEvent)); + this.actor.connect('motion-event', Lang.bind(this, this._onMotionEvent)); I don't think this is right - it means that opening the menu and moving the pointer over always moves the slider, even when it is not grabbed. Maybe for this kind of functionality (one-click open and select) it is better to move the handle only at the end. @@ +503,3 @@ this._dragging = false; + + this.connect('activate', Lang.bind(this, function(actor, event) { You should override 'activate' vfunc instead. Or even better, leave activate: false and connect to actor.button-release-event.
(In reply to comment #8) > I don't think this is right - it means that opening the menu and moving the > pointer over always moves the slider, even when it is not grabbed. It moves the slider _if_ BUTTON1 is pressed. I think it works quite well. > You should override 'activate' vfunc instead. I've done this. > Or even better, leave activate: > false and connect to actor.button-release-event. This wouldn't then make the menu close automatically IIUC.
Created attachment 188323 [details] [review] PopupSliderMenuItem: make slider menu items activatable
(In reply to comment #9) > (In reply to comment #8) > > I don't think this is right - it means that opening the menu and moving the > > pointer over always moves the slider, even when it is not grabbed. > > It moves the slider _if_ BUTTON1 is pressed. I think it works quite well. So I click and hold on any menu item (anywhere on the chrome, actually), move the pointer over the slider, and the slider is moved? People will start opening bug reports as soon as they discover this, it is completely counter-intuitive. > > You should override 'activate' vfunc instead. > > I've done this. > > > Or even better, leave activate: > > false and connect to actor.button-release-event. > > This wouldn't then make the menu close automatically IIUC. In fact, it shouldn't close, in the same way it doesn't close when you drag now.
(In reply to comment #11) > (In reply to comment #9) > > (In reply to comment #8) > > > I don't think this is right - it means that opening the menu and moving the > > > pointer over always moves the slider, even when it is not grabbed. > > > > It moves the slider _if_ BUTTON1 is pressed. I think it works quite well. > > So I click and hold on any menu item (anywhere on the chrome, actually), move > the pointer over the slider, and the slider is moved? Yes. If you click anywhere outside the menu it will close anyway, so I don't think it's much of a problem. I'll look at putting a grab on the output slider when the menu opens. In fact, it's a problem that this menu can have 2 sliders but the input one shouldn't bee too common. BTW, I can't find much about the design of the volume menu. This https://live.gnome.org/GnomeShell/Design/Guidelines/SystemStatus/AudioVolume seems to be quite outdated. Can you point me to something about the current design? The input slider doesn't make much sense IMHO. > People will start opening bug reports as soon as they discover this, it is > completely counter-intuitive. Since that will only happen if you click somewhere inside the menu I don't think it's much of a problem really. > > > You should override 'activate' vfunc instead. > > > > I've done this. > > > > > Or even better, leave activate: > > > false and connect to actor.button-release-event. > > > > This wouldn't then make the menu close automatically IIUC. > > In fact, it shouldn't close, in the same way it doesn't close when you drag > now. Closing after setting is how the volume widget works in Totem and in OS X's main volume thingy. I'll request designer input on this one.
(In reply to comment #12) > (In reply to comment #11) > > (In reply to comment #9) > > > (In reply to comment #8) > > > > I don't think this is right - it means that opening the menu and moving the > > > > pointer over always moves the slider, even when it is not grabbed. > > > > > > It moves the slider _if_ BUTTON1 is pressed. I think it works quite well. > > > > So I click and hold on any menu item (anywhere on the chrome, actually), move > > the pointer over the slider, and the slider is moved? > > Yes. If you click anywhere outside the menu it will close anyway, so I don't > think it's much of a problem. I'll look at putting a grab on the output slider > when the menu opens. In fact, it's a problem that this menu can have 2 sliders > but the input one shouldn't bee too common. Well, if we're going to write this for volume and volume only (which would be the only reason for accepting this regression), it is better to patch volume.js instead of popupMenu.js > > BTW, I can't find much about the design of the volume menu. This > > https://live.gnome.org/GnomeShell/Design/Guidelines/SystemStatus/AudioVolume > > seems to be quite outdated. Can you point me to something about the current > design? The input slider doesn't make much sense IMHO. There is no current design. I wrote the sound menu, some people preferred to the designed one, and it was accepted. (Btw, the input slider was present in GNOME 2 as well)
Created attachment 189436 [details] [review] volume.js: make slider menu items activatable I've patched volume.js only now. I just needed to do a slight change to PopupSliderMenuItem to be able to override the 'activate' initialization parameter.
Review of attachment 189436 [details] [review]: ::: js/ui/status/volume.js @@ +247,3 @@ + this._motionEvent(this.actor, event); + this.emit('drag-end'); + this.emit('activate', event); This behaviour is really weird: if you stop dragging above the slider the menu closes, otherwise it stays open. I think it should always stay open (given it would be difficult to make it always close)
(In reply to comment #15) > This behaviour is really weird: if you stop dragging above the slider the menu > closes, otherwise it stays open. I think it should always stay open (given it > would be difficult to make it always close) That's the entire point of this! If the user is trying to set the volume he wants to set the volume and get on with his life. He does not want to set the volume and then have to dismiss the menu.
(In reply to comment #16) > (In reply to comment #15) > > This behaviour is really weird: if you stop dragging above the slider the menu > > closes, otherwise it stays open. I think it should always stay open (given it > > would be difficult to make it always close) > > That's the entire point of this! If the user is trying to set the volume he > wants to set the volume and get on with his life. He does not want to set the > volume and then have to dismiss the menu. Yes, but he'll be very puzzled when half of times the menu closes and the other half it doesn't.
Do we still want this?
I'd say no, because the new system status menus make it much harder to know if it was intended or not.