Code review comment for lp:~rpadovani/webbrowser-app/settings-page

Revision history for this message
Riccardo Padovani (rpadovani) wrote :

Thanks for your review, Bill.

> I found some problems with the Homepage setting after clicking on it:
> 1) autocaps should be turned off on the field (Qt.ImhNoAutoUppercase)
> 2) predictive text should be turned off on the field (Qt.ImhNoPredictiveText)
> 3) set the hint for url field so the keyboard will display .com keys
> (Qt.ImhUrlCharactersOnly) - actually doing this might mean you don't need 1)
> and 2) as it might happen automatically

Right, done. I explicitly set all 3 properties

> 4) Upon displaying the dialog you should request focus on the input field with
> the caret at the end of the line. This will cause osk to appear by default
> saving the user an extra click

Right, done

> 5) after making a change to a new site and pressing "Save", it is not saved
> and does not appear in the Homepage field in the main Settings.

Sorry, I'm not able to reproduce that, works fine here.
Do you have any log? Does it happens only in some conditions?

> There is no visual difference between "Privacy" and "Reset browser settings",
> but one brings you to another page (Privacy) and the other executes an action
> immediately. I believe Privacy should have a expansion arrow (like Privacy >)
> such that it's clear it leads you to another page.

While I agree with you on that, Oliver and I chose to stick to design for the first implementation: we will probably remove also the divider after the last element, but at the moment we follow that slide: https://docs.google.com/presentation/d/1woHjO8K4iqyVZZlfQ4BXL0DhYbwkEmZ7wvcUhYzHDRk/edit#slide=id.g354125fb9_030

Of course, I can implement that if you know (not only for Privacy but also for search engines), just let me know.

« Back to merge proposal