widelands-dev team mailing list archive

[GunChleoc <fios@xxxxxxxxxxxxxxxxxxx>]

Thanks for the review :)

Will need to keep an eye out for the greying out thing when I add the other dropdowns.

Diff comments:

> 
> === modified file 'src/ui_basic/dropdown.cc'
> --- src/ui_basic/dropdown.cc	2017-02-25 11:17:28 +0000
> +++ src/ui_basic/dropdown.cc	2017-03-29 16:32:24 +0000
> @@ -46,63 +48,104 @@
>                             int32_t y,
>                             uint32_t w,
>                             uint32_t h,
> +                           int button_dimension,
>                             const std::string& label,
> +                           const DropdownType type,
>                             const Image* background,
>                             const Image* button_background)
>     : UI::Panel(parent,
>                 x,
>                 y,
> -               w,
> -               base_height()),  // Height only to fit the button, so we can use this in Box layout.
> +               type == DropdownType::kTextual ? w : button_dimension,
> +               // Height only to fit the button, so we can use this in Box layout.
> +               base_height(button_dimension)),
>       max_list_height_(h - 2 * get_h()),
> +     list_width_(w),
> +     button_dimension_(button_dimension),
>       mouse_tolerance_(50),
>       button_box_(this, 0, 0, UI::Box::Horizontal, w, h),
> -     push_button_(&button_box_,
> -                  "dropdown_select",
> -                  0,
> -                  0,
> -                  24,
> -                  get_h(),
> -                  button_background,
> -                  g_gr->images().get("images/ui_basic/scrollbar_down.png"),
> -                  pgettext("dropdown", "Select Item")),
> -     display_button_(&button_box_, "dropdown_label", 0, 0, w - 24, get_h(), background, label),
> -     // Hook into parent so we can drop down outside the panel
> -     list_(parent, x, y + get_h(), w, 0, button_background, ListselectLayout::kDropdown),
> -     label_(label) {
> -	list_.set_visible(false);
> -	list_.set_background(background);
> -	display_button_.set_perm_pressed(true);
> +     push_button_(type == DropdownType::kTextual ?
> +                     new UI::Button(&button_box_,
> +                                    "dropdown_select",
> +                                    0,
> +                                    0,
> +                                    button_dimension,
> +                                    get_h(),
> +                                    button_background,
> +                                    g_gr->images().get("images/ui_basic/scrollbar_down.png"),
> +                                    pgettext("dropdown", "Select Item")) :
> +                     nullptr),
> +     display_button_(&button_box_,
> +                     "dropdown_label",
> +                     0,
> +                     0,
> +                     type == DropdownType::kTextual ? w - button_dimension : button_dimension,
> +                     get_h(),
> +                     background,
> +                     label),
> +     label_(label),
> +     type_(type),
> +     is_enabled_(true) {
> +	assert(max_list_height_ > 0);
> +	// Hook into highest parent that we can get so that we can drop down outside the panel.
> +	// Positioning breaks down with TabPanels, so we exclude them.
> +	while (parent->get_parent() && !is_a(UI::TabPanel, parent->get_parent())) {
> +		parent = parent->get_parent();
> +	}
> +	list_ = new UI::Listselect<uintptr_t>(
> +	   parent, 0, 0, w, 0, button_background, ListselectLayout::kDropdown);
> +
> +	list_->set_visible(false);
> +	list_->set_background(background);
> +
>  	button_box_.add(&display_button_);
> -	button_box_.add(&push_button_);
> +	display_button_.sigclicked.connect(boost::bind(&BaseDropdown::toggle_list, this));
> +	if (push_button_ != nullptr) {
> +		display_button_.set_perm_pressed(true);
> +		button_box_.add(push_button_);
> +		push_button_->sigclicked.connect(boost::bind(&BaseDropdown::toggle_list, this));
> +	}
>  	button_box_.set_size(w, get_h());
> -
> -	display_button_.sigclicked.connect(boost::bind(&BaseDropdown::toggle_list, this));
> -	push_button_.sigclicked.connect(boost::bind(&BaseDropdown::toggle_list, this));
> -	list_.clicked.connect(boost::bind(&BaseDropdown::set_value, this));
> -	list_.clicked.connect(boost::bind(&BaseDropdown::toggle_list, this));
> +	list_->clicked.connect(boost::bind(&BaseDropdown::set_value, this));
> +	list_->clicked.connect(boost::bind(&BaseDropdown::toggle_list, this));
>  	set_can_focus(true);
> +	set_value();
>  	layout();
>  }
>  
>  BaseDropdown::~BaseDropdown() {
> -	clear();
> +	// The list will clear itself

I will rephrase the comment then. The point is that calling clear() here will lead to crashes, and it took me quite a while to chase down the issue. So, I don't want anybody accidentally "fixing" this in the future.

>  }
>  
>  void BaseDropdown::set_height(int height) {
> -	max_list_height_ = height - base_height();
> +	max_list_height_ = height - base_height(button_dimension_);
>  	layout();
>  }
>  
>  void BaseDropdown::layout() {
> -	const int base_h = base_height();
> -	const int w = get_w();
> +	const int base_h = base_height(button_dimension_);
> +	const int w = type_ == DropdownType::kTextual ? get_w() : button_dimension_;
>  	button_box_.set_size(w, base_h);
> -	display_button_.set_desired_size(w - 24, base_h);
> +	display_button_.set_desired_size(
> +	   type_ == DropdownType::kTextual ? w - button_dimension_ : w, base_h);
>  	int new_list_height =
> -	   std::min(static_cast<int>(list_.size()) * list_.get_lineheight(), max_list_height_);
> -	list_.set_size(w, new_list_height);
> +	   std::min(static_cast<int>(list_->size()) * list_->get_lineheight(), max_list_height_);
> +	list_->set_size(type_ == DropdownType::kTextual ? w : list_width_, new_list_height);
>  	set_desired_size(w, base_h);
> +
> +	// Update list position.
> +	// The list is hooked into the highest parent that we can get so that we can drop down outside
> +	// the panel.
> +	// Positioning breaks down with TabPanels, so we exclude them.
> +	UI::Panel* parent = get_parent();
> +	int new_list_y = get_y() + get_h() + parent->get_y();
> +	int new_list_x = get_x() + parent->get_x();
> +	while (parent->get_parent() && !is_a(UI::TabPanel, parent->get_parent())) {
> +		parent = parent->get_parent();
> +		new_list_y += parent->get_y();
> +		new_list_x += parent->get_x();
> +	}
> +	list_->set_pos(Vector2i(new_list_x, new_list_y));
>  }
>  
>  void BaseDropdown::add(const std::string& name,


-- 
https://code.launchpad.net/~widelands-dev/widelands/bug-536489-pictorial-dropdown/+merge/319023
Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-536489-pictorial-dropdown.