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

Revision history for this message
Olivier Tilloy (osomon) wrote :

That works very nicely, good job Riccardo! I have a bunch of comments from my functional review:

 - When tapping a history entry to browse to it (or a domain entry to show the detailed view), there should be some visual feedback (the pressed state should change the background color to something darker). This is a regression introduced by this branch, afaict, and as a consequence doesn’t fix properly bug #1358676 unlike advertised.

 - The horizontal spacing between the icon and the text in the history entries is too narrow, I also think it’s a regression compared to the current trunk.

 - When swiping an entry to the right to delete it, the red confirmation button shows up on the left. I would expect that tapping anywhere else on the screen would cancel the action, but apparently only tapping on the item itself does that, which I find a bit confusing. This feels especially awkward if I swipe to delete one domain, then tap on another domain to expand it (which takes me to the detailed view), then tap "see less", I’m back at the history view and the red confirmation button is still there.

 - Long-pressing on a domain in the history view triggers the multi-selection mode, but the domain on which I long pressed isn’t initially selected, I think it should.

« Back to merge proposal