-
Notifications
You must be signed in to change notification settings - Fork 25
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
Add search bar to OSCollapsibleItemList #618
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.
Just some nitpicks. Otherwise this is great.
Also, I think these would be good improvements, either for here directly or filed as tickets for later.
- some kind of button to clear the text in the search bar would be helpful.
- Hiding the collapsible sections that didn't match at all would be make it clearer
protected: | ||
void paintEvent(QPaintEvent* event) override; | ||
|
||
private: | ||
QVBoxLayout* m_vLayout; | ||
QLineEdit* m_searchBox; |
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 was trying to see if this shouldn't be on the base class "OSItemSelector" and realize we have an unused class ModelObjectTreeWidget.
The other class that inherits it is OSItemList
. Should that one had a search bar? Not sure what it does to be honest.
QLayoutItem* layoutItem = nullptr; | ||
OSCollapsibleItem* collapsibleItem = nullptr; |
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.
These two at least can be defined and initialized in the outer for loop no?
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.
Done!
@macumber This is great! here's a Peek capture. |
I skipped adding the search bar to OSItem because that is used to display all objects of a single type. There are two instances of OSItem. One is under each OSCollapsibleItemHeader in an OSCollapsibleItemList (box on the right) so a second search bar would be redundant. The other is in the ModelObjectListView used on tabs that don't have a grid view (box on the left). We could configure that search for the case on the left but I don't think it's as useful there. Another place OSCollapsibleItemList is used is the load definition tab, not sure if we'd want to disable the search bar here for consistency with ModelObjectListView? Finally, the other place a search bar might be useful would be on the OSGridView. |
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.
Love the adjustments, thanks @macumber this is going to be very exciting for users! Here's a Peek, v2:
@@ -62,6 +62,7 @@ OSCollapsibleItemList::OSCollapsibleItemList(bool addScrollArea, QWidget* parent | |||
this->setLayout(outerVLayout); | |||
|
|||
m_searchBox = new QLineEdit(); | |||
m_searchBox->setClearButtonEnabled(true); |
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.
Lol, that icon is a broom?
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.
Hehe, weird on Windows it is a nice x button.
Adds a search bar to OSCollapsibleItemList used for most of the library tabs. OSListView which is used by the ScriptsTab is considerably different, so I did not add a search feature there for now.