[Merge] lp:~artmello/webbrowser-app/webbrowser-app-bookmarks_view into lp:webbrowser-app

Arthur Mello arthur.mello at canonical.com
Mon Sep 14 23:38:23 UTC 2015


Thanks a lot for reviewing this. Comments bellow:

> - I can not get any favicon to load, even for sites that I just visited and
> have a favicon. The "globe" icon is always shown instead

I am not able to reproduce it, it seems to be working with old bookmarks and with new ones. How are you reproducing it? Some specific url domain?

> - Open two browsers windows side by side and in one open the bookmarks view
> and in the other the history view:
>   The styles of the two are not consistent with each other. If necessary speak
> with design about this, but I would change the following things:
>   - In widescreen mode:
>     - Use the same font as the history items for bookmark items
>     - Use the same font as the dates in the history view for folders

Fonts were the same, I set it to use the same fontSize. Let me know if it is better?

>     - The bookmarks view header should have a separator
>     - The folder list items should have a separator

Done

>   - In non-widescreen mode:
>     - The bookmarks view has an header, the history doesn't. I would add one
> to the history.

Since that would be a change on history view, what  do you think of me doing this on a different MR?

>     - The folders in the bookmarks view should probably start all collapsed
> instead of all open, unless no folders exist.

Done


Today we have URlDelegate and UrlDelegateWide with a lot of duplicate code, specially the one related with the url/title font. I didnt look further but would make sense to mix both those components on the same one? What do you think?
-- 
https://code.launchpad.net/~artmello/webbrowser-app/webbrowser-app-bookmarks_view/+merge/270613
Your team Ubuntu Phablet Team is subscribed to branch lp:webbrowser-app.



More information about the Ubuntu-reviews mailing list