-
-
Notifications
You must be signed in to change notification settings - Fork 383
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Itemviews basetreeview cleanup #2464
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I noticed that the menu now breaks if there are no other versions other than the selected one. The previous implementation still showed the submenu, which I think works better.
Also if we limit the number of loaded results we should somehow still make sure that the list includes the currently selected release. It can happen that this is not the case if the total count exceeds the request limit and the current release is outside the returned result. Maybe we can inject the current release from already loaded release data?
I think the limit of 40 is still too much:
Maybe better use 20 only?
What do you mean by "it breaks"? It should be disabled (as there's no other versions).
Yes, that would be the best option, since we don't know if the currently selected will be or not in retrieved limited list of releases.
This is why I added a default value, my idea is to add an option to make it configurable (using this default value), because it is very dependent on the graphical environment (size of fonts, screen resolution, etc.). |
…ction_more_details
Those methods don't depend on anything
f97691c
to
b83c0e3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Summary
Problem
Solution
Action
Additional actions required: