Changeset View
Changeset View
Standalone View
Standalone View
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); | ||||
elvisangelaccio: I don't think this comment adds any value, code is already self-explanatory. I'd just remove it. | |||||
1339 | actionShowAllPlaces->setCheckable(true); | ||||
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 | }); | ||||
elvisangelaccio: Same, please remove it | |||||
1346 | | ||||
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 | | ||||
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… | |||||
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). | |||||
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 | | |||
1353 | void 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 |
I don't think this comment adds any value, code is already self-explanatory. I'd just remove it.