Code review comment for lp:~brandontschaefer/unity/shutdown-dialog-hi-dpi

Revision history for this message
Brandon Schaefer (brandontschaefer) wrote :

> Ops, it seems you're touching also ApplicationLauncherIcon.cpp, due to mergig
> with other fix... :)
>
> 26 + std::string const FONT = "Ubuntu Light 12";
>
> I would have preferred if you left the const TYPE as discussed...
>
>
> 42 + , texture_size_(-1)
> GetDefaultMaxTextureSize(texture_prefix);
>
> The gdk_pixbuf_get_file_info is cached anyway, so imho you don't need to track
> the texture_size_... I'd just make GetDefaultMaxTextureSize to return a
> RawPixel. Otherwise, change the Get prefix, as it might imply it would return
> something.
>

Fixed.

> 156 +#include <unity-shared/UScreen.h>
>
> I guess you don't need that anymore, right?

Fixed.

>
> 155 +#include <unity-shared/RawPixel.h>
> Also this one is now implicit by including the SessionButton

Still best to keep the header there.

>
> 225 + monitor.changed.connect([this] (bool changed) {
>
> Not really a bug as bool's are int's, but that should be an (int monitor) :)
>

Fixed.

> Running it, now works properly, but I think that at this point we should also
> scale the Close button, as it might be very tiny on HiDpi monitors (and at
> this point borders as well).

« Back to merge proposal