Adress the first round of Angelaccio's review comments

- Split the viewContainers(bool includeInActive) into two methods
    without parameters
- Prevent users from accidently hiding all Url Navigators by
    preventing the dangerous action and then displaying a helpful
    message instead
Unrelated to review comments: Remove a useless line of code
This commit is contained in:
Felix Ernst 2020-08-12 20:45:17 +02:00 committed by Elvis Angelaccio
parent 00abc6d280
commit b6fc58c3c3
5 changed files with 39 additions and 17 deletions

View file

@ -68,7 +68,7 @@ bool DolphinBookmarkHandler::supportsTabs() const
QList<KBookmarkOwner::FutureBookmark> DolphinBookmarkHandler::currentBookmarkList() const
{
const auto viewContainers = m_mainWindow->viewContainers(false);
const auto viewContainers = m_mainWindow->activeViewContainers();
QList<FutureBookmark> bookmarks;
bookmarks.reserve(viewContainers.size());
for (const auto viewContainer : viewContainers) {

View file

@ -49,6 +49,7 @@
#include <KJobWidgets>
#include <KLocalizedString>
#include <KMessageBox>
#include <KMessageWidget>
#include <KNS3/KMoreToolsMenuFactory>
#include <KProtocolInfo>
#include <KProtocolManager>
@ -202,7 +203,7 @@ DolphinMainWindow::~DolphinMainWindow()
{
}
QVector<DolphinViewContainer*> DolphinMainWindow::viewContainers(bool includeInactive) const
QVector<DolphinViewContainer *> DolphinMainWindow::activeViewContainers() const
{
QVector<DolphinViewContainer*> viewContainers;
@ -845,7 +846,8 @@ void DolphinMainWindow::showFilterBar()
void DolphinMainWindow::toggleLocationInToolbar()
{
// collect needed variables
const bool locationInToolbar = actionCollection()->action(QStringLiteral("location_in_toolbar"))->isChecked();
QAction *locationInToolbarAction = actionCollection()->action(QStringLiteral("location_in_toolbar"));
const bool locationInToolbar = locationInToolbarAction->isChecked();
auto viewContainers = this->viewContainers();
auto urlNavigatorWidgetAction = static_cast<DolphinUrlNavigatorWidgetAction *>
(actionCollection()->action(QStringLiteral("url_navigator")));
@ -856,6 +858,21 @@ void DolphinMainWindow::toggleLocationInToolbar()
const int selectionStart = lineEdit->selectionStart();
const int selectionLength = lineEdit->selectionLength();
// prevent the switching if it would leave the user without a visible UrlNavigator
if (locationInToolbar && !toolBar()->actions().contains(urlNavigatorWidgetAction)) {
QAction *configureToolbars = actionCollection()->action(KStandardAction::name(KStandardAction::ConfigureToolbars));
KMessageWidget *messageWidget = m_activeViewContainer->showMessage(
xi18nc("@info 2 is the visible text on a button just below the message",
"The location could not be moved onto the toolbar because there is currently "
"no \"%1\" item on the toolbar. Select <interface>%2</interface> and add the "
"\"%1\" item. Then this will work.", urlNavigatorWidgetAction->iconText(),
configureToolbars->iconText()), DolphinViewContainer::Information);
messageWidget->addAction(configureToolbars);
messageWidget->addAction(locationInToolbarAction);
locationInToolbarAction->setChecked(false);
return;
}
// do the switching
GeneralSettings::setLocationInToolbar(locationInToolbar);
if (locationInToolbar) {
@ -870,7 +887,7 @@ void DolphinMainWindow::toggleLocationInToolbar()
}
}
urlNavigatorWidgetAction->setUrlNavigatorVisible(!locationInToolbar);
urlNavigatorWidgetAction->setUrlNavigatorVisible(locationInToolbar);
m_activeViewContainer->urlNavigator()->setUrlEditable(isEditable);
if (hasFocus) { // the rest of this method is unneeded perfectionism
m_activeViewContainer->urlNavigator()->editor()->lineEdit()->setText(lineEdit->text());
@ -1756,8 +1773,6 @@ void DolphinMainWindow::setupActions()
"Depending on the settings this Widget is blank/invisible.",
"Url Navigator (auto-hide)"));
actionCollection()->addAction(QStringLiteral("url_navigator"), urlNavigatorWidgetAction);
connect(locationInToolbar, &KToggleAction::triggered,
urlNavigatorWidgetAction, &DolphinUrlNavigatorWidgetAction::setUrlNavigatorVisible);
// for context menu
QAction* showTarget = actionCollection()->addAction(QStringLiteral("show_target"));

View file

@ -65,17 +65,19 @@ public:
* having a split view setup, the nonactive view
* is usually shown in darker colors.
*/
DolphinViewContainer* activeViewContainer() const;
DolphinViewContainer *activeViewContainer() const;
/**
* Returns view containers for all tabs
* @param includeInactive When true all view containers available in
* this window are returned. When false the
* view containers of split views that are not
* currently active are ignored.
* Default is true.
* Returns the active view containers of all tabs.
* @see activeViewContainer()
* Use viewContainers() to also include the inactive ones.
*/
QVector<DolphinViewContainer*> viewContainers(bool includeInactive = true) const;
QVector<DolphinViewContainer *> activeViewContainers() const;
/**
* Returns all view containers.
*/
QVector<DolphinViewContainer *> viewContainers() const;
/**
* Opens each directory in \p dirs in a separate tab. If \a splitView is set,

View file

@ -367,10 +367,13 @@ void DolphinViewContainer::disconnectUrlNavigator()
updateNavigatorWidgetVisibility();
}
void DolphinViewContainer::showMessage(const QString& msg, MessageType type)
KMessageWidget *DolphinViewContainer::showMessage(const QString& msg, MessageType type)
{
if (msg.isEmpty()) {
return;
return m_messageWidget;
}
for (auto action : m_messageWidget->actions()) {
m_messageWidget->removeAction(action);
}
m_messageWidget->setText(msg);
@ -396,6 +399,7 @@ void DolphinViewContainer::showMessage(const QString& msg, MessageType type)
m_messageWidget->hide();
}
m_messageWidget->animatedShow();
return m_messageWidget;
}
void DolphinViewContainer::readSettings()

View file

@ -140,8 +140,9 @@ public:
/**
* Shows the message \msg with the given type non-modal above
* the view-content.
* @return the KMessageWidget used to show the message
*/
void showMessage(const QString& msg, MessageType type);
KMessageWidget *showMessage(const QString& msg, MessageType type);
/**
* Refreshes the view container to get synchronized with the (updated) Dolphin settings.