1231388 - [GUI] use a datepicker as opposed to entering date manually

Status: RESOLVED FIXED

(Testing :: mozregression, defect)

Description

[William Lachance (:wlach)]

Right now you have to enter the date manually (i.e. 2015-11-24) in the field when starting a bisection. It would be nice if we could use some kind of datepicker widget to do this instead.

Something like this: http://jqueryui.com/datepicker/ (obviously you'd be using the Qt equivalent, that's for the web)

Comment 1

[Vaibhav Tulsyan [:xenny]]

I think we can use QCalendarWidget (http://pyqt.sourceforge.net/Docs/PyQt4/qcalendarwidget.html) for this. Thoughts, @parkouss & @wlach?

Comment 2

[Saurabh Singhal [:saurabhs]]

Attachment: date-picker.png

This is how it looks with my current changes. Should I try to make it exactly like http://jqueryui.com/datepicker/. I meant, having a text box and when we click on it, calendar pops up ?

Comment 3

[Julien Pagès (:parkouss)]

Oh, I was thinking about yet another dialog (in addition to the textbox) - but this looks quite cool to me!

Will, what do you think ? Look at the attachment above.

Comment 4

[William Lachance (:wlach)]

Yeah, this looks good to me! I'd put the dropdown selecting type beside "Last known good" and "First known bad" though.

Text nits:

1. Change 'Bisection range selection' to just 'Range selection'
2. This is probably a longstanding issue, but I'd change 'Search fix' to 'Search for fix'.

Comment 5

[Saurabh Singhal [:saurabhs]]

Attachment: date-picker-1.png

How does this look @parkouss and @wlach?

Comment 6

[Julien Pagès (:parkouss)]

Looks pretty good to me, thanks!

I have to test it on windows to see how it looks like though, since most of our users are on windows. I'll do that once you are happy with the code and ask for a review.

Comment 7

[Saurabh Singhal [:saurabhs]]

Attachment: date-picker-2.png

Aligned the comboboxes with "First known bad build" and "Last known good build" labels.

Comment 8

[Saurabh Singhal [:saurabhs]]

Attachment: Pull Request

Comment 9

[Julien Pagès (:parkouss)]

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

So, this looks very good - this is really an improvement for the date selection! I love this calendar widget.

Though there is one problem, if I change the selection method from date to changeset for example, the QLineEdit used for the changeset takes the same place as the calendar - and this is quite horrible. :(

I believe this can be fixed quite easily, quick search on internet:

http://stackoverflow.com/questions/14480696/resize-qstackedwidget-to-the-page-which-is-opened#14485901

Tell me if you also have the same behavior (I'm running Linux/Gnome3) - then please fix this and ask again for a review. Once fixed it will be awesome!

Comment 10

[Saurabh Singhal [:saurabhs]]

Yes, I am also noticing the same behavior on OSX. Terribly sorry for not checking this properly. Will fix it.

Comment 11

[Julien Pagès (:parkouss)]

Attachment: screen01.png

Comment 12

[Julien Pagès (:parkouss)]

No problem! Thanks for looking at it. :)

Comment 13

[Saurabh Singhal [:saurabhs]]

Attachment: date-picker-3.png

I have been able to fix it but not completely. What is happening is that when you select 'release' for the first time, it displays that huge combobox. But, if you select 'release', then some other option, say 'build' and then again 'release', it displays the nice small combobox.

I tried multiple things like subclassing QStackedWidget and overriding sizeHint() and minimumSizeHint(). I also tried to play around with setSizePolicy() and the adjustSize() methods. But this is one place where I am stuck. Any suggestions?

Comment 14

[Julien Pagès (:parkouss)]

Hmm, hard to tell without seeing any code - but it looks like maybe the signal slot connection to resize the currentChange widget was done at a wrong time (since it seems to work for every but the first time).

Try debugging it - maybe some print statements to be sure it is really doing what you think - then show us some code if you're stuck. :)

Comment 15

[Saurabh Singhal [:saurabhs]]

Attachment: Pull Request

Comment 16

[Julien Pagès (:parkouss)]

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

Unfortunately, if you choose changeset, then go back to date, the widget uses a minimum size... Also resizing the dialog break everything again. :(

I quickly tried this:

http://stackoverflow.com/a/14486499

This works, but is a bit brutal as it move every other widget along. But this is acceptable - take a look at it if you want, or try something else!

Thanks for sticking with it. :)

Comment 17

[Saurabh Singhal [:saurabhs]]

I am confused as I am not seeing the behavior you mention.

When I change the from changeset to date, it works fine for me.

Also, I don't understand the resizing the dialog part. Should everything scale to a proper size as the dialog is resized?

Comment 18

[Julien Pagès (:parkouss)]

Attachment: date-changeset-date.png

this is what I have if I select changeset, then date again.

Comment 19

[Julien Pagès (:parkouss)]

Attachment: after-resizing.png

and here, I choose changeset, the resize the dialog.

Comment 20

[Julien Pagès (:parkouss)]

(In reply to Julien Pagès (:parkouss) from comment #19)
> Created attachment 8701829 [details]
> after-resizing.png
> 
> and here, I choose changeset, the resize the dialog.

I mean I choose release - but same thing I guess.

Comment 21

[Saurabh Singhal [:saurabhs]]

I am also seeing the issue you mentioned on Ubuntu. However, I tried both the accepted solution and the solution you mentioned on the linked SO question. None of them seemed to work for me. Any ideas?

Comment 22

[Julien Pagès (:parkouss)]

(In reply to Saurabh Singhal from comment #21)
> I am also seeing the issue you mentioned on Ubuntu. However, I tried both
> the accepted solution and the solution you mentioned on the linked SO
> question. None of them seemed to work for me. Any ideas?

Hm, the solution on SO worked for me - it has to be done in the slot each time the widget changes. I can show you the code if you need, tell me.

Comment 23

[Saurabh Singhal [:saurabhs]]

Attachment: datepicker.patch

So, here's the code I tried. The approaches:

1. Uncommented code: the accepted solution on SO.

2. Commented code: the solution you mentioned. Also, for this code, I used self.stacked.currentChanged.connect(self.onCurrentChanged).

So, what needs to be changed here?

Comment 24

[Julien Pagès (:parkouss)]

Attachment: working.diff

There is a couple of issues in your implementation from SO. Here is a working patch for me, tell me if that works.

Comment 25

[Saurabh Singhal [:saurabhs]]

Attachment: datepickerUbuntu.png

I'm sorry but this doesn't work for me. When we select date->some other field->date, the calendar almost disappears.

Comment 26

[Julien Pagès (:parkouss)]

(In reply to Saurabh Singhal from comment #25)
> Created attachment 8702111 [details]
> datepickerUbuntu.png
> 
> I'm sorry but this doesn't work for me. When we select date->some other
> field->date, the calendar almost disappears.

No problem! Well, I don't see this. I applied this patch on top of your branch crisron:date-picker (no other modification) and it works fine. Though, I'm using gnome 3.18.3 on archlinux, maybe it is different from your system/gnome version and could explain the difference.

A last idea that comes to mind would be to put in the stacked widget some wrapper widget for the QlineEdit fields, that would use vertical spacers to force the widget to not be expendable. A good thing would be to create a new .ui file to create the interface of RangeSelection, then adapt the RangeSelection class to use this new .ui file.

Does that make sense to you ?

Comment 27

[Saurabh Singhal [:saurabhs]]

Being a beginner in PyQt, I don't understand much of what you are proposing. Some questions:

1. What wrapper widget would I use? How?

2. In the new .ui file, should I create all the widgets I use in RangeSelection? Would I need to create a python file for this too?

Comment 28

[Julien Pagès (:parkouss)]

Oh, I guess the questions here have been answered on irc. Saurabh, are you still working on this bug ?

Comment 29

[Saurabh Singhal [:saurabhs]]

Yes. There has been a lot of office work for the past 2 weeks. So, I was not able to give this bug much time. I plan to finish it up this weekend.

Comment 30

[Saurabh Singhal [:saurabhs]]

Attachment: Pull Request

Comment 31

[Julien Pagès (:parkouss)]

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

Great stuff.

And thanks a lot for all the renaming of range to build!

I just noted a small nit in the PR, could you fix it please ? this is about the use of self.helperUI. Once fixed, I'll be happy to merge this!

Just ping me when you're done - thanks!

Comment 32

[Julien Pagès (:parkouss)]

Landed in https://github.com/mozilla/mozregression/commit/db49b843074772679db7cf4cf60afca800f7b084.