Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/scrollbar_beautification into lp:widelands

2016-11-14 Thread GunChleoc 
Yes, that's on purpose. We need new art.

https://wl.widelands.org/forum/topic/2648/

Thanks for the review

@bunnybot merge
-- 
https://code.launchpad.net/~widelands-dev/widelands/scrollbar_beautification/+merge/309635
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/scrollbar_beautification.

___
Mailing list: https://launchpad.net/~widelands-dev
Post to : widelands-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~widelands-dev
More help   : https://help.launchpad.net/ListHelp

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/scrollbar_beautification into lp:widelands

2016-11-14 Thread SirVer 
Review: Approve

lgtm.

Unrelated: have you started the game at 800x600 lately? The 'Widelands' logo is 
partly covered by the edge images.

Diff comments:

> 
> === modified file 'src/ui_basic/table.cc'
> --- src/ui_basic/table.cc 2016-10-24 14:04:00 +
> +++ src/ui_basic/table.cc 2016-11-03 07:34:45 +
> @@ -523,6 +543,34 @@
>   return columns_[sort_column_].compare(a, b);
>  }
>  
> +void Table::layout() {
> + // If we have a flexible column, adjust the column sizes.
> + if (flexible_column_ != std::numeric_limits::max()) {

Instead of if (){} use if (!()) {return}; to avoid right drift.

> + int all_columns_width = scrollbar_->is_enabled() ? 
> scrollbar_->get_w() : 0;
> + for (const auto& column : columns_) {
> + all_columns_width += column.width;
> + }
> + if (all_columns_width != get_w()) {
> + Column& column = columns_.at(flexible_column_);
> + column.width = column.width + get_w() - 
> all_columns_width;
> + column.btn->set_size(column.width, column.btn->get_h());
> + int offset = 0;
> + for (const auto& col : columns_) {
> + col.btn->set_pos(Vector2i(offset, 
> col.btn->get_y()));
> + offset = col.btn->get_x() + col.btn->get_w();
> + }
> + if (scrollbar_->is_enabled()) {
> + const UI::Button* last_column_btn = 
> columns_.back().btn;
> + scrollbar_filler_button_->set_pos(
> +Vector2i(last_column_btn->get_x() + 
> last_column_btn->get_w(), 0));
> + scrollbar_filler_button_->set_visible(true);
> + } else {
> + scrollbar_filler_button_->set_visible(false);
> + }
> + }
> + }
> +}
> +
>  /**
>   * Sort the table alphabetically. Make sure that the current selection stays
>   * valid (though it might scroll out of visibility).


-- 
https://code.launchpad.net/~widelands-dev/widelands/scrollbar_beautification/+merge/309635
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/scrollbar_beautification.

___
Mailing list: https://launchpad.net/~widelands-dev
Post to : widelands-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~widelands-dev
More help   : https://help.launchpad.net/ListHelp

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/scrollbar_beautification into lp:widelands

2016-10-30 Thread Tino 
Review: Approve

Everything's ok on my end.
-- 
https://code.launchpad.net/~widelands-dev/widelands/scrollbar_beautification/+merge/309635
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/scrollbar_beautification.

___
Mailing list: https://launchpad.net/~widelands-dev
Post to : widelands-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~widelands-dev
More help   : https://help.launchpad.net/ListHelp

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/scrollbar_beautification into lp:widelands

2016-10-30 Thread Klaus Halfmann 
Review: Needs Fixing compile

Mhh, this does not compile for me:

In file included from 
...widelands-repo/scrollbar_beautification/src/wui/encyclopedia_window.h:32:
...widelands-repo/scrollbar_beautification/src/ui_basic/table.h:352:7: error: 
no matching constructor for
  initialization of 'Table'
   : Base(parent, x, y, w, h, descending) {


-- 
https://code.launchpad.net/~widelands-dev/widelands/scrollbar_beautification/+merge/309635
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/scrollbar_beautification.

___
Mailing list: https://launchpad.net/~widelands-dev
Post to : widelands-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~widelands-dev
More help   : https://help.launchpad.net/ListHelp