Skip to content

Commit

Permalink
Stacked Widget: some fixes and cleanup to select_layer
Browse files Browse the repository at this point in the history
First off, this reverts the change from a9ba9a9. A detailed explanation has been added to the class
member documentation.

Secondly, this removes calls to set_visible. The generator should handle this internally. It seems the previous
code rendered manual calls necessary. This is no longer the case.

Lastly, this fixes an issue where selecting all layers (-1) did not always result in the top-most layer receiving
events. It appears by some quirk of the generator system, only the last "selected" layer receives events, even
if multiple are visible, *not* simply the top-most one in the stack.
  • Loading branch information
Vultraz committed Mar 26, 2017
1 parent 701649a commit f980fe6
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 26 deletions.
50 changes: 29 additions & 21 deletions src/gui/widgets/stacked_widget.cpp
Expand Up @@ -33,7 +33,7 @@ REGISTER_WIDGET(stacked_widget)

stacked_widget::stacked_widget()
: container_base(1)
, generator_(generator_base::build(true, false, generator_base::independent, false))
, generator_(generator_base::build(false, false, generator_base::independent, false))
, selected_layer_(-1)
{
}
Expand Down Expand Up @@ -117,34 +117,42 @@ void stacked_widget::set_self_active(const bool /*active*/)
/* DO NOTHING */
}

void stacked_widget::select_layer_internal(const unsigned int layer, const bool select) const
{
// Selecting a layer that's already selected appears to actually deselect
// it, so make sure to only perform changes we want.
if(generator_->is_selected(layer) != select) {
generator_->select_item(layer, select);
}
}

void stacked_widget::select_layer(const int layer)
{
const unsigned int num_layers = generator_->get_item_count();
selected_layer_ = std::max(-1, std::min<int>(layer, num_layers - 1));

// Deselect all layers except the chosen one.
for(unsigned int i = 0; i < num_layers; ++i) {
if(selected_layer_ >= 0) {
const bool selected = i == static_cast<unsigned int>(selected_layer_);
// Select current layer, leave the rest unselected.
select_layer_internal(i, selected);
generator_->item(i).set_visible(selected
? widget::visibility::visible
: widget::visibility::hidden);
} else {
// Select everything.
select_layer_internal(i, true);
generator_->item(i).set_visible(widget::visibility::visible);
const bool selected = i == static_cast<unsigned int>(selected_layer_);

/* Selecting a previously selected item will deselect it, regardless of the what is passed to
* select_item. This causes issues if this function is called when all layers are visible (for
* example, initialization). For layers other than the chosen one, this is the desired behavior.
* However the chosen layer could *also* be deselected undesirably due to the conditions outlined
* above, and as this widget's generator does not stipulate a minimum selection, it's possible to
* end up with no layers visible at all.
*
* This works around that by performing no selection unless necessary to change states.
*/
if(generator_->is_selected(i) != selected) {
generator_->select_item(i, selected);
}
}

// If we already have our chosen layer, exit.
if(selected_layer_ >= 0) {
return;
}

// Else, re-show all layers.
for(unsigned int i = 0; i < num_layers; ++i) {
/* By design, only the top-most item will receive events even if multiple items are visible.
* Additionally, if this point is reached, all layers have already been hidden by the loop above,
* so no check on an item's selected state is necessary; just select them all.
*/
generator_->select_item(i, true);
}
}

unsigned int stacked_widget::get_layer_count() const
Expand Down
16 changes: 11 additions & 5 deletions src/gui/widgets/stacked_widget.hpp
Expand Up @@ -90,6 +90,17 @@ class stacked_widget : public container_base
* The pointer is not owned by this class, it's stored in the content_grid_
* of the scrollbar_container super class and freed when it's grid is
* freed.
*
* NOTE: the generator is initialized with has_minimum (first arg) as false,
* which seems a little counter-intuitive at first. After all, shouldn't the
* stack always have at least one layer visible? However, this allows select_layer
* to function correctly.
*
* If has_minimum is true, the generator policy selected (one_item) can leave
* multiple layers selected when selecting a new one. This is most likely due to
* cases where the new chosen layer comes *after* the currently selected one.
* In that case, the generator would not allow the interim state where no layer
* before the new chosen layer is reached in the loop.
*/
generator_base* generator_;

Expand All @@ -98,11 +109,6 @@ class stacked_widget : public container_base
*/
int selected_layer_;

/**
* Helper to ensure the correct state is set when selecting a layer.
*/
void select_layer_internal(const unsigned int layer, const bool select) const;

/** See @ref styled_widget::get_control_type. */
virtual const std::string& get_control_type() const override;

Expand Down

0 comments on commit f980fe6

Please sign in to comment.