Changeset View
Changeset View
Standalone View
Standalone View
src/dolphinviewcontainer.cpp
| Show First 20 Lines • Show All 45 Lines • ▼ Show 20 Line(s) | |||||
| 46 | #include <QMimeData> | 46 | #include <QMimeData> | ||
| 47 | #include <QTimer> | 47 | #include <QTimer> | ||
| 48 | #include <QUrl> | 48 | #include <QUrl> | ||
| 49 | #include <QVBoxLayout> | 49 | #include <QVBoxLayout> | ||
| 50 | 50 | | |||
| 51 | DolphinViewContainer::DolphinViewContainer(const QUrl& url, QWidget* parent) : | 51 | DolphinViewContainer::DolphinViewContainer(const QUrl& url, QWidget* parent) : | ||
| 52 | QWidget(parent), | 52 | QWidget(parent), | ||
| 53 | m_topLayout(nullptr), | 53 | m_topLayout(nullptr), | ||
| 54 | m_navigatorWidget(nullptr), | ||||
| 54 | m_urlNavigator(nullptr), | 55 | m_urlNavigator(nullptr), | ||
| 56 | m_emptyTrashButton(nullptr), | ||||
| 55 | m_searchBox(nullptr), | 57 | m_searchBox(nullptr), | ||
| 56 | m_messageWidget(nullptr), | 58 | m_messageWidget(nullptr), | ||
| 57 | m_view(nullptr), | 59 | m_view(nullptr), | ||
| 58 | m_filterBar(nullptr), | 60 | m_filterBar(nullptr), | ||
| 59 | m_statusBar(nullptr), | 61 | m_statusBar(nullptr), | ||
| 60 | m_statusBarTimer(nullptr), | 62 | m_statusBarTimer(nullptr), | ||
| 61 | m_statusBarTimestamp(), | 63 | m_statusBarTimestamp(), | ||
| 62 | m_autoGrabFocus(true) | 64 | m_autoGrabFocus(true) | ||
| 63 | #ifdef KActivities_FOUND | 65 | #ifdef KActivities_FOUND | ||
| 64 | , m_activityResourceInstance(0) | 66 | , m_activityResourceInstance(0) | ||
| 65 | #endif | 67 | #endif | ||
| 66 | { | 68 | { | ||
| 67 | hide(); | 69 | hide(); | ||
| 68 | 70 | | |||
| 69 | m_topLayout = new QVBoxLayout(this); | 71 | m_topLayout = new QVBoxLayout(this); | ||
| 70 | m_topLayout->setSpacing(0); | 72 | m_topLayout->setSpacing(0); | ||
| 71 | m_topLayout->setMargin(0); | 73 | m_topLayout->setMargin(0); | ||
| 72 | 74 | | |||
| 75 | m_navigatorWidget = new QWidget(this); | ||||
| 76 | QHBoxLayout* navigatorLayout = new QHBoxLayout(m_navigatorWidget); | ||||
| 77 | navigatorLayout->setSpacing(0); | ||||
| 78 | navigatorLayout->setMargin(0); | ||||
| 79 | | ||||
| 73 | m_urlNavigator = new KUrlNavigator(new KFilePlacesModel(this), url, this); | 80 | m_urlNavigator = new KUrlNavigator(new KFilePlacesModel(this), url, this); | ||
| 74 | connect(m_urlNavigator, &KUrlNavigator::activated, | 81 | connect(m_urlNavigator, &KUrlNavigator::activated, | ||
| 75 | this, &DolphinViewContainer::activate); | 82 | this, &DolphinViewContainer::activate); | ||
| 76 | connect(m_urlNavigator->editor(), &KUrlComboBox::completionModeChanged, | 83 | connect(m_urlNavigator->editor(), &KUrlComboBox::completionModeChanged, | ||
| 77 | this, &DolphinViewContainer::saveUrlCompletionMode); | 84 | this, &DolphinViewContainer::saveUrlCompletionMode); | ||
| 78 | 85 | | |||
| 79 | const GeneralSettings* settings = GeneralSettings::self(); | 86 | const GeneralSettings* settings = GeneralSettings::self(); | ||
| 80 | m_urlNavigator->setUrlEditable(settings->editableUrl()); | 87 | m_urlNavigator->setUrlEditable(settings->editableUrl()); | ||
| 81 | m_urlNavigator->setShowFullPath(settings->showFullPath()); | 88 | m_urlNavigator->setShowFullPath(settings->showFullPath()); | ||
| 82 | m_urlNavigator->setHomeUrl(Dolphin::homeUrl()); | 89 | m_urlNavigator->setHomeUrl(Dolphin::homeUrl()); | ||
| 83 | KUrlComboBox* editor = m_urlNavigator->editor(); | 90 | KUrlComboBox* editor = m_urlNavigator->editor(); | ||
| 84 | editor->setCompletionMode(KCompletion::CompletionMode(settings->urlCompletionMode())); | 91 | editor->setCompletionMode(KCompletion::CompletionMode(settings->urlCompletionMode())); | ||
| 85 | 92 | | |||
| 93 | m_emptyTrashButton = new QPushButton(QIcon::fromTheme(QStringLiteral("user-trash")), "&Empty Trash", this); | ||||
rkflx: Won't do a full code review for Dolphin related patches, but this should just use the same icon… | |||||
yeah, listen to Henrik here. This was my bad; we have since determined that we should not be using the -symbolic icons. See D10770#213782 ngraham: yeah, listen to Henrik here. This was my bad; we have since determined that we should //not//… | |||||
…because only few icon sets contain this icon. Nevertheless, the HiDPI problem still stands. Perhaps as a workaround we can replicate D10843, i.e. use another icon (providing the same line weight, which -symbolic lacks) and add a fallback. However, better to use user-trash here, and do the workaround in a follow-up patch (to keep git log clean and change it everywhere at once). rkflx: …because only few icon sets contain this icon. Nevertheless, the HiDPI problem still stands. | |||||
| 94 | m_emptyTrashButton->setFlat(true); | ||||
| 95 | connect(m_emptyTrashButton, &QPushButton::clicked, this, [this]() { Trash::empty(this); }); | ||||
markg: Trash::instance().empty(this); -> Trash::empty(this); | |||||
markg: add "this" before the lambda (context thingy). | |||||
| 96 | connect(&Trash::instance(), &Trash::emptinessChanged, m_emptyTrashButton, &QPushButton::setDisabled); | ||||
| 97 | m_emptyTrashButton->setDisabled(Trash::isEmpty()); | ||||
markg: Trash::instance().isEmpty() -> Trash::isEmpty() | |||||
| 98 | m_emptyTrashButton->hide(); | ||||
| 99 | | ||||
| 86 | m_searchBox = new DolphinSearchBox(this); | 100 | m_searchBox = new DolphinSearchBox(this); | ||
| 87 | m_searchBox->hide(); | 101 | m_searchBox->hide(); | ||
| 88 | connect(m_searchBox, &DolphinSearchBox::activated, this, &DolphinViewContainer::activate); | 102 | connect(m_searchBox, &DolphinSearchBox::activated, this, &DolphinViewContainer::activate); | ||
| 89 | connect(m_searchBox, &DolphinSearchBox::closeRequest, this, &DolphinViewContainer::closeSearchBox); | 103 | connect(m_searchBox, &DolphinSearchBox::closeRequest, this, &DolphinViewContainer::closeSearchBox); | ||
| 90 | connect(m_searchBox, &DolphinSearchBox::searchRequest, this, &DolphinViewContainer::startSearching); | 104 | connect(m_searchBox, &DolphinSearchBox::searchRequest, this, &DolphinViewContainer::startSearching); | ||
| 91 | connect(m_searchBox, &DolphinSearchBox::returnPressed, this, &DolphinViewContainer::requestFocus); | 105 | connect(m_searchBox, &DolphinSearchBox::returnPressed, this, &DolphinViewContainer::requestFocus); | ||
| 92 | 106 | | |||
| 93 | m_messageWidget = new KMessageWidget(this); | 107 | m_messageWidget = new KMessageWidget(this); | ||
| ▲ Show 20 Lines • Show All 48 Lines • ▼ Show 20 Line(s) | |||||
| 142 | #if KIO_VERSION >= QT_VERSION_CHECK(5, 37, 0) | 156 | #if KIO_VERSION >= QT_VERSION_CHECK(5, 37, 0) | ||
| 143 | m_view->dropUrls(destination, event, m_urlNavigator->dropWidget()); | 157 | m_view->dropUrls(destination, event, m_urlNavigator->dropWidget()); | ||
| 144 | #else | 158 | #else | ||
| 145 | // TODO: remove as soon as we can hard-depend of KF5 >= 5.37 | 159 | // TODO: remove as soon as we can hard-depend of KF5 >= 5.37 | ||
| 146 | m_view->dropUrls(destination, event, m_view); | 160 | m_view->dropUrls(destination, event, m_view); | ||
| 147 | #endif | 161 | #endif | ||
| 148 | }); | 162 | }); | ||
| 149 | 163 | | |||
| 164 | connect(m_view, &DolphinView::directoryLoadingCompleted, this, [this](){ | ||||
markg: add "this" before the lambda (context thingy). | |||||
| 165 | if (m_view->url().scheme() == QLatin1String("trash")) { | ||||
| 166 | m_emptyTrashButton->show(); | ||||
| 167 | } else { | ||||
| 168 | m_emptyTrashButton->hide(); | ||||
| 169 | } | ||||
How about m_emptyTrashButton->setVisible(m_view->url().scheme() == QLatin1String("trash")) ? elvisangelaccio: How about `m_emptyTrashButton->setVisible(m_view->url().scheme() == QLatin1String("trash"))` ? | |||||
| 170 | }); | ||||
| 171 | | ||||
| 150 | // Initialize status bar | 172 | // Initialize status bar | ||
| 151 | m_statusBar = new DolphinStatusBar(this); | 173 | m_statusBar = new DolphinStatusBar(this); | ||
| 152 | m_statusBar->setUrl(m_view->url()); | 174 | m_statusBar->setUrl(m_view->url()); | ||
| 153 | m_statusBar->setZoomLevel(m_view->zoomLevel()); | 175 | m_statusBar->setZoomLevel(m_view->zoomLevel()); | ||
| 154 | connect(m_view, &DolphinView::urlChanged, | 176 | connect(m_view, &DolphinView::urlChanged, | ||
| 155 | m_statusBar, &DolphinStatusBar::setUrl); | 177 | m_statusBar, &DolphinStatusBar::setUrl); | ||
| 156 | connect(m_view, &DolphinView::zoomLevelChanged, | 178 | connect(m_view, &DolphinView::zoomLevelChanged, | ||
| 157 | m_statusBar, &DolphinStatusBar::setZoomLevel); | 179 | m_statusBar, &DolphinStatusBar::setZoomLevel); | ||
| Show All 21 Lines | |||||
| 179 | connect(m_filterBar, &FilterBar::filterChanged, | 201 | connect(m_filterBar, &FilterBar::filterChanged, | ||
| 180 | this, &DolphinViewContainer::setNameFilter); | 202 | this, &DolphinViewContainer::setNameFilter); | ||
| 181 | connect(m_filterBar, &FilterBar::closeRequest, | 203 | connect(m_filterBar, &FilterBar::closeRequest, | ||
| 182 | this, &DolphinViewContainer::closeFilterBar); | 204 | this, &DolphinViewContainer::closeFilterBar); | ||
| 183 | connect(m_filterBar, &FilterBar::focusViewRequest, | 205 | connect(m_filterBar, &FilterBar::focusViewRequest, | ||
| 184 | this, &DolphinViewContainer::requestFocus); | 206 | this, &DolphinViewContainer::requestFocus); | ||
| 185 | connect(m_view, &DolphinView::urlChanged, | 207 | connect(m_view, &DolphinView::urlChanged, | ||
| 186 | m_filterBar, &FilterBar::slotUrlChanged); | 208 | m_filterBar, &FilterBar::slotUrlChanged); | ||
| 187 | 209 | | |||
This syntax is a little unusual. Can you create a separate function that can be connected here rather than defining it inline? ngraham: This syntax is a little unusual. Can you create a separate function that can be connected here… | |||||
It's C++ lambda function from C++11. Very handy for small tasks. Pros: don't pollute namespace with additional definitions, it takes less code. rominf: It's C++ lambda function from C++11. Very handy for small tasks. Pros: don't pollute namespace… | |||||
I'll let the more experienced Dolphin developers weigh in here, but generally it's best to follow the existing coding style as long as there's nothing explicitly wrong with it, rather than being clever and saving code. Code is cheap! ngraham: I'll let the more experienced Dolphin developers weigh in here, but generally it's best to… | |||||
| 188 | m_topLayout->addWidget(m_urlNavigator); | 210 | navigatorLayout->addWidget(m_urlNavigator); | ||
| 211 | navigatorLayout->addWidget(m_emptyTrashButton); | ||||
| 212 | | ||||
| 213 | m_topLayout->addWidget(m_navigatorWidget); | ||||
| 189 | m_topLayout->addWidget(m_searchBox); | 214 | m_topLayout->addWidget(m_searchBox); | ||
| 190 | m_topLayout->addWidget(m_messageWidget); | 215 | m_topLayout->addWidget(m_messageWidget); | ||
| 191 | m_topLayout->addWidget(m_view); | 216 | m_topLayout->addWidget(m_view); | ||
| 192 | m_topLayout->addWidget(m_filterBar); | 217 | m_topLayout->addWidget(m_filterBar); | ||
| 193 | m_topLayout->addWidget(m_statusBar); | 218 | m_topLayout->addWidget(m_statusBar); | ||
| 194 | 219 | | |||
| 195 | setSearchModeEnabled(isSearchUrl(url)); | 220 | setSearchModeEnabled(isSearchUrl(url)); | ||
| 196 | 221 | | |||
| ▲ Show 20 Lines • Show All 135 Lines • ▼ Show 20 Line(s) | 356 | if (enabled == isSearchModeEnabled()) { | |||
| 332 | if (enabled && !m_searchBox->hasFocus()) { | 357 | if (enabled && !m_searchBox->hasFocus()) { | ||
| 333 | m_searchBox->setFocus(); | 358 | m_searchBox->setFocus(); | ||
| 334 | m_searchBox->selectAll(); | 359 | m_searchBox->selectAll(); | ||
| 335 | } | 360 | } | ||
| 336 | return; | 361 | return; | ||
| 337 | } | 362 | } | ||
| 338 | 363 | | |||
| 339 | m_searchBox->setVisible(enabled); | 364 | m_searchBox->setVisible(enabled); | ||
| 340 | m_urlNavigator->setVisible(!enabled); | 365 | m_navigatorWidget->setVisible(!enabled); | ||
Where did: go to? Is there a reason for that? markg: Where did:
m_urlNavigator->setVisible(!enabled);
go to? Is there a reason for that? | |||||
Now m_urlNavigator is contained in m_navigatorWidget. m_navigatorWidget is a droped-in replacement to m_urlNavigator. That means all operations with m_urlNavigator use m_navigatorWidget. rominf: Now `m_urlNavigator` is contained in `m_navigatorWidget`. `m_navigatorWidget` is a droped-in… | |||||
| 341 | 366 | | |||
| 342 | if (enabled) { | 367 | if (enabled) { | ||
| 343 | const QUrl& locationUrl = m_urlNavigator->locationUrl(); | 368 | const QUrl& locationUrl = m_urlNavigator->locationUrl(); | ||
| 344 | m_searchBox->fromSearchUrl(locationUrl); | 369 | m_searchBox->fromSearchUrl(locationUrl); | ||
| 345 | } else { | 370 | } else { | ||
| 346 | m_view->setViewPropertiesContext(QString()); | 371 | m_view->setViewPropertiesContext(QString()); | ||
| 347 | 372 | | |||
| 348 | // Restore the URL for the URL navigator. If Dolphin has been | 373 | // Restore the URL for the URL navigator. If Dolphin has been | ||
| ▲ Show 20 Lines • Show All 352 Lines • Show Last 20 Lines | |||||
Won't do a full code review for Dolphin related patches, but this should just use the same icon as everywhere in Dolphin: user-trash.