Differential D13174 Diff 35635 src/IncrementalSearchBar.cpp

Changeset View

Standalone View

View Options

src/IncrementalSearchBar.cpp

Show First 20 Lines • Show All 51 Lines • ▼ Show 20 Line(s)50{
52 52​
53 auto closeButton = new QToolButton(this); 53​ auto closeButton = new QToolButton(this);
54 closeButton->setObjectName(QStringLiteral("close-button")); 54​ closeButton->setObjectName(QStringLiteral("close-button"));
55 closeButton->setToolTip(i18nc("@info:tooltip", "Close the search bar")); 55​ closeButton->setToolTip(i18nc("@info:tooltip", "Close the search bar"));
56 closeButton->setAutoRaise(true); 56​ closeButton->setAutoRaise(true);
57 closeButton->setIcon(QIcon::fromTheme(QStringLiteral("dialog-close"))); 57​ closeButton->setIcon(QIcon::fromTheme(QStringLiteral("dialog-close")));
58 connect(closeButton, &QToolButton::clicked, this, &Konsole::IncrementalSearchBar::closeClicked); 58​ connect(closeButton, &QToolButton::clicked, this, &Konsole::IncrementalSearchBar::closeClicked);
59 59​
60 QLabel *findLabel = new QLabel(i18nc("@label:textbox", "Find:"), this); ​
61 _searchEdit = new QLineEdit(this); 60​ _searchEdit = new QLineEdit(this);
62 _searchEdit->setClearButtonEnabled(true); 61​ _searchEdit->setClearButtonEnabled(true);
63 _searchEdit->installEventFilter(this); 62​ _searchEdit->installEventFilter(this);
63​ _searchEdit->setPlaceholderText(i18nc("@label:textbox", "Find:"));
safaalfulaijUnsubmitted
Done

Very small note: "Find…" :)
Semicolons are for labels, placeholder text uses three dots.

Not sure about all the string freeze thing though…

safaalfulaij: Very small note: "Find…" :) Semicolons are for labels, placeholder text uses three dots. Not…
64 _searchEdit->setObjectName(QStringLiteral("search-edit")); 64​ _searchEdit->setObjectName(QStringLiteral("search-edit"));
65 _searchEdit->setToolTip(i18nc("@info:tooltip", "Enter the text to search for here")); 65​ _searchEdit->setToolTip(i18nc("@info:tooltip", "Enter the text to search for here"));
66 66​
67 // text box may be a minimum of 6 characters wide and a maximum of 10 characters wide 67​ // text box may be a minimum of 6 characters wide and a maximum of 10 characters wide
68 // (since the maxWidth metric is used here, more characters probably will fit in than 6 68​ // (since the maxWidth metric is used here, more characters probably will fit in than 6
69 // and 10) 69​ // and 10)
70 QFontMetrics metrics(_searchEdit->font()); 70​ QFontMetrics metrics(_searchEdit->font());
71 int maxWidth = metrics.maxWidth(); 71​ int maxWidth = metrics.maxWidth();
72 _searchEdit->setMinimumWidth(maxWidth * 6); 72​ _searchEdit->setMinimumWidth(maxWidth * 6);
73 _searchEdit->setMaximumWidth(maxWidth * 10); 73​ _searchEdit->setMaximumWidth(maxWidth * 10);
74 74​
75 _searchTimer = new QTimer(this); 75​ _searchTimer = new QTimer(this);
76 _searchTimer->setInterval(250); 76​ _searchTimer->setInterval(250);
77 _searchTimer->setSingleShot(true); 77​ _searchTimer->setSingleShot(true);
78 connect(_searchTimer, &QTimer::timeout, this, 78​ connect(_searchTimer, &QTimer::timeout, this,
79 &Konsole::IncrementalSearchBar::notifySearchChanged); 79​ &Konsole::IncrementalSearchBar::notifySearchChanged);
80 connect(_searchEdit, &QLineEdit::textChanged, _searchTimer, 80​ connect(_searchEdit, &QLineEdit::textChanged, _searchTimer,
81 static_cast<void (QTimer::*)()>(&QTimer::start)); 81​ static_cast<void (QTimer::*)()>(&QTimer::start));
82 82​
83 _findNextButton = new QToolButton(this); 83​ _findNextButton = new QToolButton(this);
84 _findNextButton->setObjectName(QStringLiteral("find-next-button")); 84​ _findNextButton->setObjectName(QStringLiteral("find-next-button"));
85 _findNextButton->setText(i18nc("@action:button Go to the next phrase", "Next")); 85​ _findNextButton->setText(i18nc("@action:button Go to the next phrase", "Next"));
86 _findNextButton->setToolButtonStyle(Qt::ToolButtonTextBesideIcon); 86​ _findNextButton->setToolButtonStyle(Qt::ToolButtonIconOnly);
87​ _findNextButton->setAutoRaise(true);
88​
rizzitelloUnsubmitted
Not Done

extra new line

rizzitello: extra new line
87 _findNextButton->setToolTip(i18nc("@info:tooltip", 89​ _findNextButton->setToolTip(i18nc("@info:tooltip",
88 "Find the next match for the current search phrase")); 90​ "Find the next match for the current search phrase"));
89 connect(_findNextButton, &QToolButton::clicked, this, 91​ connect(_findNextButton, &QToolButton::clicked, this,
90 &Konsole::IncrementalSearchBar::findNextClicked); 92​ &Konsole::IncrementalSearchBar::findNextClicked);
91 93​
92 _findPreviousButton = new QToolButton(this); 94​ _findPreviousButton = new QToolButton(this);
95​ _findPreviousButton->setAutoRaise(true);
96​
rizzitelloUnsubmitted
Not Done

extra new line

rizzitello: extra new line
93 _findPreviousButton->setObjectName(QStringLiteral("find-previous-button")); 97​ _findPreviousButton->setObjectName(QStringLiteral("find-previous-button"));
94 _findPreviousButton->setText(i18nc("@action:button Go to the previous phrase", "Previous")); 98​ _findPreviousButton->setText(i18nc("@action:button Go to the previous phrase", "Previous"));
95 _findPreviousButton->setToolButtonStyle(Qt::ToolButtonTextBesideIcon); 99​ _findPreviousButton->setToolButtonStyle(Qt::ToolButtonIconOnly);
96 _findPreviousButton->setToolTip(i18nc("@info:tooltip", 100​ _findPreviousButton->setToolTip(i18nc("@info:tooltip",
97 "Find the previous match for the current search phrase")); 101​ "Find the previous match for the current search phrase"));
98 connect(_findPreviousButton, &QToolButton::clicked, this, 102​ connect(_findPreviousButton, &QToolButton::clicked, this,
99 &Konsole::IncrementalSearchBar::findPreviousClicked); 103​ &Konsole::IncrementalSearchBar::findPreviousClicked);
100 104​
101 _searchFromButton = new QToolButton(this); 105​ _searchFromButton = new QToolButton(this);
106​ _searchFromButton->setAutoRaise(true);
107​
rizzitelloUnsubmitted
Not Done

Extra new line

rizzitello: Extra new line
102 _searchFromButton->setObjectName(QStringLiteral("search-from-button")); 108​ _searchFromButton->setObjectName(QStringLiteral("search-from-button"));
103 connect(_searchFromButton, &QToolButton::clicked, this, 109​ connect(_searchFromButton, &QToolButton::clicked, this,
104 &Konsole::IncrementalSearchBar::searchFromClicked); 110​ &Konsole::IncrementalSearchBar::searchFromClicked);
105 111​
106 auto optionsButton = new QToolButton(this); 112​ auto optionsButton = new QToolButton(this);
107 optionsButton->setObjectName(QStringLiteral("find-options-button")); 113​ optionsButton->setObjectName(QStringLiteral("find-options-button"));
108 optionsButton->setText(i18nc("@action:button Display options menu", "Options")); ​
109 optionsButton->setCheckable(false); 114​ optionsButton->setCheckable(false);
110 optionsButton->setPopupMode(QToolButton::InstantPopup); 115​ optionsButton->setPopupMode(QToolButton::InstantPopup);
111 optionsButton->setArrowType(Qt::DownArrow); 116​ optionsButton->setArrowType(Qt::NoArrow);
112 optionsButton->setToolButtonStyle(Qt::ToolButtonTextOnly); 117​ optionsButton->setToolButtonStyle(Qt::ToolButtonIconOnly);
ahmadsamirUnsubmitted
Done

This is redundant as Qt::NoArrow is the default for QToolButton: http://doc.qt.io/qt-5/qtoolbutton.html#arrowType-prop

ahmadsamir: This is redundant as Qt::NoArrow is the default for QToolButton: http://doc.qt.io/qt…
113 optionsButton->setToolTip(i18nc("@info:tooltip", "Display the options menu")); 118​ optionsButton->setToolTip(i18nc("@info:tooltip", "Display the options menu"));
119​ optionsButton->setIcon(QIcon::fromTheme(QStringLiteral("configure")));
120​ optionsButton->setAutoRaise(true);
114 121​
115 barLayout->addWidget(closeButton); ​
116 barLayout->addWidget(findLabel); ​
117 barLayout->addWidget(_searchEdit); 122​ barLayout->addWidget(_searchEdit);
118 barLayout->addWidget(_findNextButton); 123​ barLayout->addWidget(_findNextButton);
119 barLayout->addWidget(_findPreviousButton); 124​ barLayout->addWidget(_findPreviousButton);
120 barLayout->addWidget(_searchFromButton); 125​ barLayout->addWidget(_searchFromButton);
121 barLayout->addWidget(optionsButton); 126​ barLayout->addWidget(optionsButton);
127​ barLayout->addWidget(closeButton);
122 128​
123 // Fill the options menu 129​ // Fill the options menu
124 auto optionsMenu = new QMenu(this); 130​ auto optionsMenu = new QMenu(this);
125 optionsButton->setMenu(optionsMenu); 131​ optionsButton->setMenu(optionsMenu);
126 132​
127 _caseSensitive = optionsMenu->addAction(i18nc("@item:inmenu", "Case sensitive")); 133​ _caseSensitive = optionsMenu->addAction(i18nc("@item:inmenu", "Case sensitive"));
128 _caseSensitive->setCheckable(true); 134​ _caseSensitive->setCheckable(true);
129 _caseSensitive->setToolTip(i18nc("@info:tooltip", "Sets whether the search is case sensitive")); 135​ _caseSensitive->setToolTip(i18nc("@info:tooltip", "Sets whether the search is case sensitive"));
Show All 16 Lines
146 _reverseSearch->setCheckable(true); 152​ _reverseSearch->setCheckable(true);
147 _reverseSearch->setToolTip(i18nc("@info:tooltip", 153​ _reverseSearch->setToolTip(i18nc("@info:tooltip",
148 "Sets whether search should start from the bottom")); 154​ "Sets whether search should start from the bottom"));
149 connect(_reverseSearch, &QAction::toggled, this, 155​ connect(_reverseSearch, &QAction::toggled, this,
150 &Konsole::IncrementalSearchBar::updateButtonsAccordingToReverseSearchSetting); 156​ &Konsole::IncrementalSearchBar::updateButtonsAccordingToReverseSearchSetting);
151 updateButtonsAccordingToReverseSearchSetting(); 157​ updateButtonsAccordingToReverseSearchSetting();
152 setOptions(); 158​ setOptions();
153 159​
154 barLayout->addStretch(); 160​ barLayout->setContentsMargins(0, 0, 0, 0);
155 161​ barLayout->setMargin(0);
156 barLayout->setContentsMargins(4, 4, 4, 4); 162​ barLayout->setSpacing(0);
157 163​
158 setLayout(barLayout); 164​ setLayout(barLayout);
165​
rizzitelloUnsubmitted
Not Done

Extra new line

rizzitello: Extra new line
166​ adjustSize();
167​ setAutoFillBackground(true);
159} 168​}
160 169​
161void IncrementalSearchBar::notifySearchChanged() 170​void IncrementalSearchBar::notifySearchChanged()
162{ 171​{
163 emit searchChanged(searchText()); 172​ emit searchChanged(searchText());
164} 173​}
165 174​
166void IncrementalSearchBar::updateButtonsAccordingToReverseSearchSetting() 175​void IncrementalSearchBar::updateButtonsAccordingToReverseSearchSetting()
167{ 176​{
168 Q_ASSERT(_reverseSearch); 177​ Q_ASSERT(_reverseSearch);
169 if (_reverseSearch->isChecked()) { 178​ if (_reverseSearch->isChecked()) {
170 _searchFromButton->setText(i18nc("@action:button Search from bottom", "From bottom")); ​
171 _searchFromButton->setToolTip(i18nc("@info:tooltip", 179​ _searchFromButton->setToolTip(i18nc("@info:tooltip",
172 "Search for the current search phrase from the bottom")); 180​ "Search for the current search phrase from the bottom"));
173 _findNextButton->setIcon(QIcon::fromTheme(QStringLiteral("go-up-search"))); 181​ _searchFromButton->setIcon(QIcon::fromTheme(QStringLiteral("go-bottom")));
174 _findPreviousButton->setIcon(QIcon::fromTheme(QStringLiteral("go-down-search"))); 182​ _findNextButton->setIcon(QIcon::fromTheme(QStringLiteral("go-previous")));
183​ _findPreviousButton->setIcon(QIcon::fromTheme(QStringLiteral("go-next")));
175 } else { 184​ } else {
176 _searchFromButton->setText(i18nc("@action:button Search from top", "From top")); ​
177 _searchFromButton->setToolTip(i18nc("@info:tooltip", 185​ _searchFromButton->setToolTip(i18nc("@info:tooltip",
178 "Search for the current search phrase from the top")); 186​ "Search for the current search phrase from the top"));
179 _findNextButton->setIcon(QIcon::fromTheme(QStringLiteral("go-down-search"))); 187​ _findNextButton->setIcon(QIcon::fromTheme(QStringLiteral("go-next")));
180 _findPreviousButton->setIcon(QIcon::fromTheme(QStringLiteral("go-up-search"))); 188​ _findPreviousButton->setIcon(QIcon::fromTheme(QStringLiteral("go-previous")));
189​ _searchFromButton->setIcon(QIcon::fromTheme(QStringLiteral("go-top")));
190​
rizzitelloUnsubmitted
Not Done

Extra new line.

rizzitello: Extra new line.
ahmadsamirUnsubmitted
Done

There is indeed an unnecessary extra new line here.

_searchFromButton->setIcon() should be in the same order in the if{}else{}, right after the _searchFrom->setToolTip() call, and before the other two setIcon() calls, just so that it's all consistent.

go-previous and go-next look a bit wrong, as they are left/right pointing arrows, respectively, and ideally one searches up/down in a document... etc (it's more logical somehow :)). IINM you changed the icons so as to differentiate between the searchFromButton arrow icon the other two arrow icons; but I suggest using go-up and go-down, respectively.

ahmadsamir: There is indeed an unnecessary extra new line here. _searchFromButton->setIcon() should be in…
181 } 191​ }
182} 192​}
183 193​
184QString IncrementalSearchBar::searchText() 194​QString IncrementalSearchBar::searchText()
185{ 195​{
186 return _searchEdit->text(); 196​ return _searchEdit->text();
187} 197​}
188 198​
▲ Show 20 Lines • Show All 140 Lines • Show Last 20 Lines