Differential D16767 Diff 45650 src/dolphinmainwindow.cpp

Changeset View

Standalone View

View Options

src/dolphinmainwindow.cpp

Show First 20 Lines • Show All 1329 Lines • ▼ Show 20 Line(s)1299#endif
1330 connect(this, &DolphinMainWindow::settingsChanged, 1330​ connect(this, &DolphinMainWindow::settingsChanged,
1331 m_placesPanel, &PlacesPanel::readSettings); 1331​ m_placesPanel, &PlacesPanel::readSettings);
1332 connect(m_placesPanel, &PlacesPanel::storageTearDownRequested, 1332​ connect(m_placesPanel, &PlacesPanel::storageTearDownRequested,
1333 this, &DolphinMainWindow::slotStorageTearDownFromPlacesRequested); 1333​ this, &DolphinMainWindow::slotStorageTearDownFromPlacesRequested);
1334 connect(m_placesPanel, &PlacesPanel::storageTearDownExternallyRequested, 1334​ connect(m_placesPanel, &PlacesPanel::storageTearDownExternallyRequested,
1335 this, &DolphinMainWindow::slotStorageTearDownExternallyRequested); 1335​ this, &DolphinMainWindow::slotStorageTearDownExternallyRequested);
1336 m_tabWidget->slotPlacesPanelVisibilityChanged(m_placesPanel->isVisible()); 1336​ m_tabWidget->slotPlacesPanelVisibilityChanged(m_placesPanel->isVisible());
1337 1337​
1338​ auto actionShowAllPlaces = new QAction(QIcon::fromTheme(QStringLiteral("hint")), i18nc("@item:inmenu", "Show Hidden Places"), this);
elvisangelaccioUnsubmitted
Done

I don't think this comment adds any value, code is already self-explanatory. I'd just remove it.

elvisangelaccio: I don't think this comment adds any value, code is already self-explanatory. I'd just remove it.
1339​ actionShowAllPlaces->setCheckable(true);
elvisangelaccioUnsubmitted
Done

Please add this as parent, otherwise it will leak (see D16817).

elvisangelaccio: Please add `this` as parent, otherwise it will leak (see D16817).
1340​ actionShowAllPlaces->setDisabled(true);
1341​
1342​ connect(actionShowAllPlaces, &QAction::triggered, this, [actionShowAllPlaces, this](bool checked){
1343​ actionShowAllPlaces->setIcon(QIcon::fromTheme(checked ? QStringLiteral("visibility") : QStringLiteral("hint")));
1344​ m_placesPanel->showHiddenEntries(checked);
1345​ });
elvisangelaccioUnsubmitted
Done

Same, please remove it

elvisangelaccio: Same, please remove it
1346​
elvisangelaccioUnsubmitted
Done

We don't need to capture this here.

elvisangelaccio: We don't need to capture `this` here.
1347​ connect(m_placesPanel, &PlacesPanel::showHiddenEntriesChanged, this, [actionShowAllPlaces] (bool checked){
1348​ actionShowAllPlaces->setChecked(checked);
1349​ actionShowAllPlaces->setIcon(QIcon::fromTheme(checked ? QStringLiteral("visibility") : QStringLiteral("hint")));
1350​ });
1351​
ngrahamUnsubmitted
Done

We don't make menu items conditionally disappear. Please only disable it when it can't be used, don't hide it.

ngraham: We don't make menu items conditionally disappear. Please only disable it when it can't be used…
elvisangelaccioUnsubmitted
Done

Well, we already do for the action in the places panel context menu (same in the KIO one). Shouldn't we be consistent with that?

elvisangelaccio: Well, we already do for the action in the places panel context menu (same in the KIO one).
ngrahamUnsubmitted
Done

Ah yes, we should fix those as well. We always try to disable inapplicable items rather than hiding them. The HIG says this, but it's kind of hidden away in https://hig.kde.org/patterns/content/settings.html?highlight=inapplicable. I'll make that more obvious on the https://hig.kde.org/components/navigation/menubar.html and https://hig.kde.org/components/navigation/contextmenu.html pages, too.

ngraham: Ah yes, we should fix those as well. We always try to disable inapplicable items rather than…
1338 // Add actions into the "Panels" menu 1352​ // Add actions into the "Panels" menu
1339 KActionMenu* panelsMenu = new KActionMenu(i18nc("@action:inmenu View", "Panels"), this); 1353​ KActionMenu* panelsMenu = new KActionMenu(i18nc("@action:inmenu View", "Panels"), this);
1340 actionCollection()->addAction(QStringLiteral("panels"), panelsMenu); 1354​ actionCollection()->addAction(QStringLiteral("panels"), panelsMenu);
1341 panelsMenu->setDelayed(false); 1355​ panelsMenu->setDelayed(false);
1342 const KActionCollection* ac = actionCollection(); 1356​ const KActionCollection* ac = actionCollection();
1343 panelsMenu->addAction(ac->action(QStringLiteral("show_places_panel"))); 1357​ panelsMenu->addAction(ac->action(QStringLiteral("show_places_panel")));
1344#ifdef HAVE_BALOO 1358​#ifdef HAVE_BALOO
1345 panelsMenu->addAction(ac->action(QStringLiteral("show_information_panel"))); 1359​ panelsMenu->addAction(ac->action(QStringLiteral("show_information_panel")));
1346#endif 1360​#endif
1347 panelsMenu->addAction(ac->action(QStringLiteral("show_folders_panel"))); 1361​ panelsMenu->addAction(ac->action(QStringLiteral("show_folders_panel")));
1348 panelsMenu->addAction(ac->action(QStringLiteral("show_terminal_panel"))); 1362​ panelsMenu->addAction(ac->action(QStringLiteral("show_terminal_panel")));
1349 panelsMenu->addSeparator(); 1363​ panelsMenu->addSeparator();
1364​ panelsMenu->addAction(actionShowAllPlaces);
1350 panelsMenu->addAction(lockLayoutAction); 1365​ panelsMenu->addAction(lockLayoutAction);
1366​
1367​ connect(panelsMenu->menu(), &QMenu::aboutToShow, this, [actionShowAllPlaces, this]{
1368​ actionShowAllPlaces->setEnabled(m_placesPanel->hiddenListCount());
1369​ });
1351} 1370​}
1352 1371​
1353void DolphinMainWindow::updateEditActions() 1372​void DolphinMainWindow::updateEditActions()
1354{ 1373​{
1355 const KFileItemList list = m_activeViewContainer->view()->selectedItems(); 1374​ const KFileItemList list = m_activeViewContainer->view()->selectedItems();
1356 if (list.isEmpty()) { 1375​ if (list.isEmpty()) {
1357 stateChanged(QStringLiteral("has_no_selection")); 1376​ stateChanged(QStringLiteral("has_no_selection"));
1358 } else { 1377​ } else {
▲ Show 20 Lines • Show All 232 Lines • Show Last 20 Lines