Code looks OK (as far as I can grasp it).
One nit inline.
I will however compile this and do some stress tesing on OSX with a dual
Monitor layout.
Will/Hsould the 'f' (or some) shortcut be supported inside the menu?
Diff comments:
> === modified file 'src/ui_basic/checkbox.cc'
> --- src/ui_basic/checkbox.cc 2016-10-25 08:11:31 +0000
> +++ src/ui_basic/checkbox.cc 2016-12-10 10:30:36 +0000
> @@ -45,33 +47,47 @@
> set_size(w, h);
> set_can_focus(true);
> set_flags(Has_Custom_Picture, true);
> - pic_graphics_ = pic;
> }
>
> Statebox::Statebox(Panel* const parent,
> Vector2i const p,
> const std::string& label_text,
> const std::string& tooltip_text,
> - uint32_t width)
> - : Panel(parent, p.x, p.y, kStateboxSize, kStateboxSize, tooltip_text),
> + int width)
> + : Panel(parent, p.x, p.y, std::max(width, kStateboxSize), kStateboxSize,
> tooltip_text),
> flags_(Is_Enabled),
> - rendered_text_(label_text.empty() ? nullptr :
> -
> UI::g_fh1->render(as_uifont(label_text),
> - width >
> (kStateboxSize + kPadding) ?
> - width -
> kStateboxSize - kPadding :
> - 0)) {
> - pic_graphics_ =
> g_gr->images().get("images/ui_basic/checkbox_light.png");
> - if (rendered_text_) {
> - int w = rendered_text_->width() + kPadding +
> pic_graphics_->width() / 2;
> - int h = std::max(rendered_text_->height(),
> pic_graphics_->height());
this is a rather complex ? expression, cans this be refactores into some helper
fucntions?
> + pic_graphics_(g_gr->images().get("images/ui_basic/checkbox_light.png")),
> + label_text_(label_text),
> + rendered_text_(nullptr) {
> + set_flags(Has_Text, !label_text_.empty());
> + layout();
> +}
> +
> +void Statebox::layout() {
> + // We only need to relayout if we have text
> + if (flags_ & Has_Text) {
> + int w = get_w();
> + int h = kStateboxSize;
> + int pic_width = kStateboxSize;
> + if (pic_graphics_) {
> + w = std::max(pic_graphics_->width(), w);
> + h = pic_graphics_->height();
> + pic_width = pic_graphics_->width();
> + }
> + rendered_text_ = label_text_.empty() ?
> + nullptr :
> + UI::g_fh1->render(
> + as_uifont(label_text_),
> + get_w() > (pic_width + kPadding) ?
> get_w() - pic_width - kPadding : 0);
> + if (rendered_text_) {
> + w = std::max(rendered_text_->width() + kPadding +
> pic_width, w);
> + h = std::max(rendered_text_->height(), h);
> + }
> set_desired_size(w, h);
> set_size(w, h);
> }
> }
>
> -Statebox::~Statebox() {
> -}
> -
> /**
> * Set the enabled state of the checkbox. A disabled checkbox cannot be
> clicked
> * and is somewhat darker to tell it apart from enabled ones.
--
https://code.launchpad.net/~widelands-dev/widelands/fsmenu_fullscreen_4_options/+merge/312966
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/fsmenu_fullscreen_4_options into lp:widelands.
_______________________________________________
Mailing list: https://launchpad.net/~widelands-dev
Post to : [email protected]
Unsubscribe : https://launchpad.net/~widelands-dev
More help : https://help.launchpad.net/ListHelp
