On 29-4-2011 15:03, venom00 wrote:
>> Style and form comments are always welcome but please give a
>> little attention to the content too:
>> - Alternative ideas to highlight matching widgets? Currently
>> they become red.
>> - Ideas on how to make the research faster? In the next patch
>> I'll try to add a little delay.
>
> Here's the new patch. I've added a 300 ms delay, now it should be more
> responsive as the search it's not done for each charatecter in input.
>
> venom00
>
> Index: src/frontends/qt4/GuiDocument.h
> ===================================================================
> --- src/frontends/qt4/GuiDocument.h (revisione 38474)
> +++ src/frontends/qt4/GuiDocument.h (copia locale)
> @@ -77,6 +77,7 @@
> void updateDefaultFormat();
> void updatePagestyle(std::string const &, std::string const &);
> bool isChildIncluded(std::string const &);
> + void hideView();
>
> ///
> BufferParams const & params() const { return bp_; }
> Index: src/frontends/qt4/GuiDocument.cpp
> ===================================================================
> --- src/frontends/qt4/GuiDocument.cpp (revisione 38474)
> +++ src/frontends/qt4/GuiDocument.cpp (copia locale)
> @@ -2139,6 +2139,12 @@
> includeonlys_.end(), child) != includeonlys_.end());
> }
>
> +void GuiDocument::hideView()
> +{
> + Dialog::hideView();
> + // Reset the search box
> + this->docPS->resetSearch();
> +}
>
> void GuiDocument::applyView()
> {
hideView() doesn't work as this is only called when the Ok or Close
button are pressed. Not when the dialog is canceled by Escape.
Maybe you should put the resetSearch function in a showEvent function
which is called when the dialog is shown.
However, you should then set the text in the textfield and then
manually reset all widgets to enabled. This means you have to either
block the signals, but better .. listen to textEdited instead of
textChanged.
Style: remove the this-> and the comment is not necessary: resetSearch()
is clear enough to me.
> +
> + connect(search_, SIGNAL(textChanged(QString)), this,
> SLOT(filterChanged(QString)));
textChanged -> textEdited so that this will not react to
programmatically changing the text in the search box as a
consequence of resetSearch() for instance.
> @@ -132,22 +163,138 @@
> QTreeWidgetItem * previous)
> {
> // do nothing when clicked on whitespace (item=NULL)
> - if( !item )
> + if(!item)
> return;
I believe this comes from my first patch to LyX some 2.5 years ago,
but if you care about the whitespaces there should also be a space
between if and (.
>
> +bool matches(QString & input, QString const & search)
QString const & input ... ?
> +{
> + // Check if the input contains the search string
> + return input.remove('&').contains(search, Qt::CaseInsensitive);
> +}
>
> +void PanelStack::searchTimeout()
I would like it more that searchTimeout just calls
search() which then implements the searching.
Then you can call search() from showEvent as well for instance.
> +{
> + QString search = search_->text();
> + bool enable_all = search.isEmpty();
> +
> + // If the search string is empty we renable all the items
renable -> enable
> + // otherwise we disable everything and then selectively
> + // re-enable matching items
> + foreach (QTreeWidgetItem * tree_item, panel_map_) {
> + setTreeItemStatus(tree_item, enable_all);
> + }
if enable_all is true, we don't need to loop over all widgets...
> +
> + foreach (QTreeWidgetItem * tree_item, panel_map_) {
> +
> + // Current widget
> + QWidget * pane_widget = widget_map_[tree_item];
> +
> + // First of all we look in the pane name
> + bool pane_matches = tree_item->text(0).contains(search,
> Qt::CaseInsensitive);
> +
> + // If the tree item has an associated pane
> + if (pane_widget) {
> [..]
> +> + }
> +
> + }
> +}
> +
Vincent
