Skip to content

Commit

Permalink
Listbox: don't try to keep the selected item visible (#3016)
Browse files Browse the repository at this point in the history 
@Vultraz stated in Discord that list box should keep the scroll position
instead.

Resolves #3016.
  • Loading branch information
@jyrkive
jyrkive committed May 19, 2018
1 parent 1caadc2 commit c364952
Showing 1 changed file with 0 additions and 9 deletions.
9 changes: 0 additions & 9 deletions src/gui/widgets/listbox.cpp
View file
Expand Up @@ -704,15 +704,6 @@ void listbox::layout_children(const bool force)

content_grid()->set_visible_rectangle(visible);

if(selected_item != -1) {
SDL_Rect rect = generator_->item(selected_item).get_rectangle();

rect.x = visible.x;
rect.w = visible.w;

show_content_rect(rect);
}

need_layout_ = false;
set_is_dirty(true);
}
Expand Down

5 comments on commit c364952

@CelticMinstrel
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is definitely better than before, but I think the best behaviour would be something like... the first visible game that still exists remains visible. In other words... if a game is removed from near the beginning, the scroll position is adjusted so that the same games are still visible.

This could be a lot of work though, especially if you stick to the method of rebuilding the entire view whenever it changes. (I'm pretty sure it could be possible to avoid doing that though?)

@Vultraz
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The gamelist isn’t constantly completely rebuilt.

@CelticMinstrel
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was before, right? But it doesn't matter, anyway. It doesn't really change the core of what I said there.

@gfgtdf
Copy link
Contributor

@gfgtdf gfgtdf commented on c364952 May 19, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was before, right?

i don't think so, the playerlist is complteley rebuild though, (which is probably one of the main reason for the ui lag in the mp lobby)

But i agree with celmin, on the main point.

@CelticMinstrel
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I was getting the two lists confused then, probably. (The playerlist should probably not be totally rebuilt either, if possible.)

Please sign in to comment.