Changeset View
Standalone View
kmenuedit.cpp
Show All 14 Lines | |||||
15 | * You should have received a copy of the GNU General Public License | 15 | * You should have received a copy of the GNU General Public License | ||
16 | * along with this program; if not, write to the Free Software | 16 | * along with this program; if not, write to the Free Software | ||
17 | * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. | 17 | * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. | ||
18 | * | 18 | * | ||
19 | */ | 19 | */ | ||
20 | #include "kmenuedit.h" | 20 | #include "kmenuedit.h" | ||
21 | 21 | | |||
22 | #include <QSplitter> | 22 | #include <QSplitter> | ||
23 | #include <QVBoxLayout> | ||||
24 | #include <QFrame> | ||||
23 | 25 | | |||
24 | #include <QAction> | 26 | #include <QAction> | ||
25 | #include <KActionCollection> | 27 | #include <KActionCollection> | ||
26 | #include <KActionMenu> | 28 | #include <KActionMenu> | ||
27 | #include <QIcon> | 29 | #include <QIcon> | ||
28 | #include <KLocalizedString> | 30 | #include <KLocalizedString> | ||
ngraham: Unrelated change. | |||||
IMHO it looks cleaner to separate includes into categories, which is what I did here. tuxxi: IMHO it looks cleaner to separate includes into categories, which is what I did here. | |||||
It surely is, but that's not related to this patch. In KDE, we try to make our commits as atomic as possible, and do clean-up like this separately. ngraham: It surely is, but that's not related to this patch. In KDE, we try to make our commits as… | |||||
29 | #include <KXMLGUIFactory> | 31 | #include <KXMLGUIFactory> | ||
30 | #include <KMessageBox> | 32 | #include <KMessageBox> | ||
31 | #include <KService> | 33 | #include <KService> | ||
32 | #include <KStandardAction> | 34 | #include <KStandardAction> | ||
33 | #include <KStandardShortcut> | 35 | #include <KStandardShortcut> | ||
36 | #include <KTreeWidgetSearchLine> | ||||
34 | #include <sonnet/configdialog.h> | 37 | #include <sonnet/configdialog.h> | ||
35 | 38 | | |||
36 | #include "treeview.h" | 39 | #include "treeview.h" | ||
37 | #include "basictab.h" | 40 | #include "basictab.h" | ||
38 | #include "preferencesdlg.h" | 41 | #include "preferencesdlg.h" | ||
39 | #include "kmenueditadaptor.h" | 42 | #include "kmenueditadaptor.h" | ||
40 | #include "configurationmanager.h" | 43 | #include "configurationmanager.h" | ||
41 | 44 | | |||
▲ Show 20 Lines • Show All 80 Lines • ▼ Show 20 Line(s) | 123 | if (newShowHiddenValue != m_showHidden) { | |||
122 | m_tree->updateTreeView(m_showHidden); | 125 | m_tree->updateTreeView(m_showHidden); | ||
123 | m_basicTab->updateHiddenEntry(m_showHidden); | 126 | m_basicTab->updateHiddenEntry(m_showHidden); | ||
124 | } | 127 | } | ||
125 | } | 128 | } | ||
126 | } | 129 | } | ||
127 | 130 | | |||
128 | void KMenuEdit::setupView() | 131 | void KMenuEdit::setupView() | ||
129 | { | 132 | { | ||
133 | // setup search and tree view | ||||
134 | m_tree = new TreeView(actionCollection(), this); | ||||
135 | | ||||
136 | m_searchLine = new KTreeWidgetSearchLine(this, m_tree); | ||||
137 | m_searchLine->setCaseSensitivity(Qt::CaseSensitivity::CaseInsensitive); | ||||
mlaurent: you can use directly Qt::CaseInsensitive | |||||
138 | m_searchLine->setKeepParentsVisible(true); | ||||
139 | m_searchLine->setPlaceholderText(i18n("Search...")); | ||||
140 | m_searchLine->setToolTip(i18n("Search through the list of applications below")); | ||||
ngraham: Close! It needs ellipses ("Search...") | |||||
Since the placeholder text says "Search..." we should probably not use the word "Filter" in the tooltip. Also "Type to" isa not really necessary since that's the only thing you can do with a search field. :) How about something like "Search through the list of applications below"? ngraham: Since the placeholder text says "Search..." we should probably not use the word "Filter" in the… | |||||
I copied the tooltip text from KOpenWithDialog, but I'll change this one. Perhaps we can change it as well tuxxi: I copied the tooltip text from KOpenWithDialog, but I'll change this one. Perhaps we can change… | |||||
ngraham: Yep, sounds like we should change it it there too! A good second patch. :) | |||||
141 | | ||||
142 | QVBoxLayout *treeLayout = new QVBoxLayout; | ||||
ngraham: No need to use `auto` here | |||||
I'm using auto to avoid duplicating the type name when using new since the type is right there; its a habit I got into since clang-tidy recommends it. I couldn't find anything in the style guides prohibiting this. If you _really_ want, I can stop using auto. tuxxi: I'm using `auto` to avoid duplicating the type name when using `new` since the type is right… | |||||
If there's nothing specifically in the style guidelines about it, it's best to follow the existing coding style. Nothing else here uses auto with new constructors, so we should follow the same style for new code. KDE software is multi-generational and it's important that each individual developer not use their own personal preferred style instead of following the existing style because that leads to the whole codebase becoming an inconsistent mess over time. Cleanup can be desirable, but that should happen separately, in its own patch, so it can be discussed on its own merits. ngraham: If there's nothing specifically in the style guidelines about it, it's best to follow the… | |||||
tuxxi: Okay, makes sense. I'll try to be more atomic :) | |||||
143 | treeLayout->addWidget(m_searchLine); | ||||
In general, we're trying to standardize on a single KDE-wide style for our search boxes:
ngraham: In general, we're trying to standardize on a single KDE-wide style for our search boxes:
- No… | |||||
144 | treeLayout->addWidget(m_tree); | ||||
145 | treeLayout->setContentsMargins(0, 0, 0, 0); // no padding, fixes alignment issues | ||||
ngraham: No need to use `auto` here | |||||
146 | QFrame *treeFrame = new QFrame; // required to insert tree + search into splitter | ||||
147 | treeFrame->setLayout(treeLayout); | ||||
148 | | ||||
130 | m_splitter = new QSplitter(this); | 149 | m_splitter = new QSplitter(this); | ||
131 | m_splitter->setOrientation(Qt::Horizontal); | 150 | m_splitter->setOrientation(Qt::Horizontal); | ||
132 | m_tree = new TreeView(actionCollection(), this); | 151 | m_splitter->addWidget(treeFrame); | ||
133 | m_splitter->addWidget(m_tree); | 152 | | ||
153 | // setup info tab view | ||||
134 | m_basicTab = new BasicTab; | 154 | m_basicTab = new BasicTab; | ||
135 | m_splitter->addWidget(m_basicTab); | 155 | m_splitter->addWidget(m_basicTab); | ||
136 | 156 | | |||
157 | // add padding to splitter | ||||
ngraham: Unrelated change | |||||
158 | m_splitter->setContentsMargins(/*left=*/5, /*top=*/0, /*right=*/5, /*bottom=*/0); | ||||
159 | | ||||
137 | connect(m_tree, SIGNAL(entrySelected(MenuFolderInfo*)), | 160 | connect(m_tree, SIGNAL(entrySelected(MenuFolderInfo*)), | ||
138 | m_basicTab, SLOT(setFolderInfo(MenuFolderInfo*))); | 161 | m_basicTab, SLOT(setFolderInfo(MenuFolderInfo*))); | ||
139 | connect(m_tree, SIGNAL(entrySelected(MenuEntryInfo*)), | 162 | connect(m_tree, SIGNAL(entrySelected(MenuEntryInfo*)), | ||
140 | m_basicTab, SLOT(setEntryInfo(MenuEntryInfo*))); | 163 | m_basicTab, SLOT(setEntryInfo(MenuEntryInfo*))); | ||
141 | connect(m_tree, &TreeView::disableAction, | 164 | connect(m_tree, &TreeView::disableAction, | ||
142 | m_basicTab, &BasicTab::slotDisableAction); | 165 | m_basicTab, &BasicTab::slotDisableAction); | ||
143 | 166 | | |||
144 | connect(m_basicTab, SIGNAL(changed(MenuFolderInfo*)), | 167 | connect(m_basicTab, SIGNAL(changed(MenuFolderInfo*)), | ||
145 | m_tree, SLOT(currentDataChanged(MenuFolderInfo*))); | 168 | m_tree, SLOT(currentDataChanged(MenuFolderInfo*))); | ||
146 | 169 | | |||
147 | connect(m_basicTab, SIGNAL(changed(MenuEntryInfo*)), | 170 | connect(m_basicTab, SIGNAL(changed(MenuEntryInfo*)), | ||
148 | m_tree, SLOT(currentDataChanged(MenuEntryInfo*))); | 171 | m_tree, SLOT(currentDataChanged(MenuEntryInfo*))); | ||
149 | 172 | | |||
150 | connect(m_basicTab, &BasicTab::findServiceShortcut, | 173 | connect(m_basicTab, &BasicTab::findServiceShortcut, | ||
151 | m_tree, &TreeView::findServiceShortcut); | 174 | m_tree, &TreeView::findServiceShortcut); | ||
152 | 175 | | |||
176 | connect(m_searchLine, &KTreeWidgetSearchLine::searchUpdated, | ||||
177 | m_tree, &TreeView::searchUpdated); | ||||
153 | // restore splitter sizes | 178 | // restore splitter sizes | ||
154 | QList<int> sizes = ConfigurationManager::getInstance()->getSplitterSizes(); | 179 | QList<int> sizes = ConfigurationManager::getInstance()->getSplitterSizes(); | ||
155 | if (sizes.isEmpty()) { | 180 | if (sizes.isEmpty()) { | ||
156 | sizes << 1 << 3; | 181 | sizes << 1 << 3; | ||
157 | } | 182 | } | ||
158 | m_splitter->setSizes(sizes); | 183 | m_splitter->setSizes(sizes); | ||
159 | m_tree->setFocus(); | 184 | m_tree->setFocus(); | ||
160 | 185 | | |||
▲ Show 20 Lines • Show All 73 Lines • Show Last 20 Lines |
Unrelated change.