Changeset View
Standalone View
sidebar/sidebar_widget.cpp
Show First 20 Lines • Show All 327 Lines • ▼ Show 20 Line(s) | |||||
328 | { | 328 | { | ||
329 | if (KMessageBox::warningContinueCancel(this, i18n("<qt>Do you really want to remove the <b>%1</b> tab?</qt>", currentButtonInfo().displayName), | 329 | if (KMessageBox::warningContinueCancel(this, i18n("<qt>Do you really want to remove the <b>%1</b> tab?</qt>", currentButtonInfo().displayName), | ||
330 | QString(), KStandardGuiItem::del()) == KMessageBox::Continue) { | 330 | QString(), KStandardGuiItem::del()) == KMessageBox::Continue) { | ||
331 | m_moduleManager.removeModule(currentButtonInfo().file); | 331 | m_moduleManager.removeModule(currentButtonInfo().file); | ||
332 | QTimer::singleShot(0, this, SLOT(updateButtons())); | 332 | QTimer::singleShot(0, this, SLOT(updateButtons())); | ||
333 | } | 333 | } | ||
334 | } | 334 | } | ||
335 | 335 | | |||
336 | void Sidebar_Widget::slotToggleShowHiddenFolders() | ||||
337 | { | ||||
338 | Q_ASSERT(currentButtonInfo().canToggleShowHiddenFolders); | ||||
339 | bool newToggleState = !currentButtonInfo().showHiddenFolders; | ||||
dfaure: This should be an assert instead, you don't create the action if canToggle... is false. | |||||
I haven't used Q_ASSERT before, and when I looked it up just now it says that all it does is print a warning message if the boolean is false. Is that what you mean? rrosch: I haven't used Q_ASSERT before, and when I looked it up just now it says that all it does is… | |||||
A failed assertion aborts the program, in debug mode. It's a way for developers to catch logic errors. dfaure: A failed assertion aborts the program, in debug mode. It's a way for developers to catch logic… | |||||
340 | m_moduleManager.setShowHiddenFolders(currentButtonInfo().file, newToggleState); | ||||
341 | // TODO: update THAT button only. | ||||
342 | QTimer::singleShot(0, this, SLOT(updateButtons())); | ||||
343 | } | ||||
344 | | ||||
336 | void Sidebar_Widget::slotMultipleViews() | 345 | void Sidebar_Widget::slotMultipleViews() | ||
337 | { | 346 | { | ||
338 | m_singleWidgetMode = !m_singleWidgetMode; | 347 | m_singleWidgetMode = !m_singleWidgetMode; | ||
339 | if ((m_singleWidgetMode) && (m_visibleViews.count() > 1)) { | 348 | if ((m_singleWidgetMode) && (m_visibleViews.count() > 1)) { | ||
340 | int tmpViewID = m_latestViewed; | 349 | int tmpViewID = m_latestViewed; | ||
341 | for (int i = 0; i < m_buttons.count(); i++) { | 350 | for (int i = 0; i < m_buttons.count(); i++) { | ||
342 | if (i != tmpViewID) { | 351 | if (i != tmpViewID) { | ||
343 | const ButtonInfo &button = m_buttons.at(i); | 352 | const ButtonInfo &button = m_buttons.at(i); | ||
▲ Show 20 Lines • Show All 58 Lines • ▼ Show 20 Line(s) | 410 | if (button.dock) { | |||
402 | m_noUpdate = true; | 411 | m_noUpdate = true; | ||
403 | if (button.dock->isVisibleTo(this)) { | 412 | if (button.dock->isVisibleTo(this)) { | ||
404 | showHidePage(i); | 413 | showHidePage(i); | ||
405 | } | 414 | } | ||
406 | delete button.module; | 415 | delete button.module; | ||
407 | delete button.dock; | 416 | delete button.dock; | ||
408 | } | 417 | } | ||
409 | m_buttonBar->removeTab(i); | 418 | m_buttonBar->removeTab(i); | ||
410 | | ||||
411 | } | 419 | } | ||
412 | m_buttons.clear(); | 420 | m_buttons.clear(); | ||
413 | 421 | | |||
414 | readConfig(); | 422 | readConfig(); | ||
415 | doLayout(); | 423 | doLayout(); | ||
416 | createButtons(); | 424 | createButtons(); | ||
417 | } | 425 | } | ||
418 | 426 | | |||
▲ Show 20 Lines • Show All 106 Lines • ▼ Show 20 Line(s) | 515 | { | |||
525 | const QUrl url(configGroup.readPathEntry("URL", QString())); | 533 | const QUrl url(configGroup.readPathEntry("URL", QString())); | ||
526 | const QString lib = configGroup.readEntry("X-KDE-KonqSidebarModule"); | 534 | const QString lib = configGroup.readEntry("X-KDE-KonqSidebarModule"); | ||
527 | const QString configOpenStr = configGroup.readEntry("Open", QString()); // NOTE: is this redundant? | 535 | const QString configOpenStr = configGroup.readEntry("Open", QString()); // NOTE: is this redundant? | ||
528 | 536 | | |||
529 | if (pos == -1) { // TODO handle insertion | 537 | if (pos == -1) { // TODO handle insertion | ||
530 | m_buttonBar->appendTab(SmallIcon(icon), lastbtn, name); | 538 | m_buttonBar->appendTab(SmallIcon(icon), lastbtn, name); | ||
531 | ButtonInfo buttonInfo(config, desktopFileName, cleanupURL(url), lib, name, icon); | 539 | ButtonInfo buttonInfo(config, desktopFileName, cleanupURL(url), lib, name, icon); | ||
532 | buttonInfo.configOpen = configGroup.readEntry("Open", false); | 540 | buttonInfo.configOpen = configGroup.readEntry("Open", false); | ||
541 | buttonInfo.canToggleShowHiddenFolders = (configGroup.readEntry("X-KDE-KonqSidebarModule", QString()) == "konqsidebar_tree"); | ||||
Why not enable the feature for *any* button that ends up creating a KonqSideBarTreeModule? Otherwise, as a user, I do "RMB / Add New / Tree Sidebar Module", configure its URL to /tmp or whatever and the feature is broken there. By not making the on/off dependent on the desktop file entry, maybe one day we can change the storing of the on/off status to be per-directory. Availability of the feature and on/off storage should be separate. dfaure: Why not enable the feature for *any* button that ends up creating a KonqSideBarTreeModule? | |||||
Sure, I can make it available for any KonqSideBarTreeModule button if you want. Will do in the next revision. rrosch: Sure, I can make it available for any KonqSideBarTreeModule button if you want. Will do in the… | |||||
542 | buttonInfo.showHiddenFolders = configGroup.readEntry("ShowHiddenFolders", false); | ||||
533 | m_buttons.insert(lastbtn, buttonInfo); | 543 | m_buttons.insert(lastbtn, buttonInfo); | ||
534 | KMultiTabBarTab *tab = m_buttonBar->tab(lastbtn); | 544 | KMultiTabBarTab *tab = m_buttonBar->tab(lastbtn); | ||
535 | tab->installEventFilter(this); | 545 | tab->installEventFilter(this); | ||
536 | connect(tab, SIGNAL(clicked(int)), this, SLOT(showHidePage(int))); | 546 | connect(tab, SIGNAL(clicked(int)), this, SLOT(showHidePage(int))); | ||
537 | 547 | | |||
538 | // Set Whats This help | 548 | // Set Whats This help | ||
539 | // This uses the comments in the .desktop files | 549 | // This uses the comments in the .desktop files | ||
540 | tab->setWhatsThis(comment); | 550 | tab->setWhatsThis(comment); | ||
Show All 17 Lines | 560 | if (bt) { | |||
558 | } | 568 | } | ||
559 | 569 | | |||
560 | if (m_currentButtonIndex > -1) { | 570 | if (m_currentButtonIndex > -1) { | ||
561 | KMenu *buttonPopup = new KMenu(this); | 571 | KMenu *buttonPopup = new KMenu(this); | ||
562 | buttonPopup->addTitle(SmallIcon(currentButtonInfo().iconName), currentButtonInfo().displayName); | 572 | buttonPopup->addTitle(SmallIcon(currentButtonInfo().iconName), currentButtonInfo().displayName); | ||
563 | buttonPopup->addAction(QIcon::fromTheme("edit-rename"), i18n("Set Name..."), this, SLOT(slotSetName())); // Item to open a dialog to change the name of the sidebar item (by Pupeno) | 573 | buttonPopup->addAction(QIcon::fromTheme("edit-rename"), i18n("Set Name..."), this, SLOT(slotSetName())); // Item to open a dialog to change the name of the sidebar item (by Pupeno) | ||
564 | buttonPopup->addAction(QIcon::fromTheme("internet-web-browser"), i18n("Set URL..."), this, SLOT(slotSetURL())); | 574 | buttonPopup->addAction(QIcon::fromTheme("internet-web-browser"), i18n("Set URL..."), this, SLOT(slotSetURL())); | ||
565 | buttonPopup->addAction(QIcon::fromTheme("preferences-desktop-icons"), i18n("Set Icon..."), this, SLOT(slotSetIcon())); | 575 | buttonPopup->addAction(QIcon::fromTheme("preferences-desktop-icons"), i18n("Set Icon..."), this, SLOT(slotSetIcon())); | ||
576 | if (currentButtonInfo().canToggleShowHiddenFolders) { | ||||
577 | QAction *toggleShowHiddenFolders = buttonPopup->addAction(i18n("Show Hidden Folders..."), this, SLOT(slotToggleShowHiddenFolders())); | ||||
The naming m_ (followed by lowercase, BTW) is for member variables. Please s/m_T/t/ and join with the next line. dfaure: The naming m_ (followed by lowercase, BTW) is for member variables.
Please s/m_T/t/ and join… | |||||
578 | toggleShowHiddenFolders->setCheckable(true); | ||||
"Hide Hidden" feels weird. Please copy Dolphin. It uses a toggle action "Show Hidden Folders", the text doesn't change, just the checkmark in front. QWidget is all single threaded, I wonder which race condition you have in mind here, please explain. dfaure: "Hide Hidden" feels weird.
Please copy Dolphin. It uses a toggle action "Show Hidden Folders"… | |||||
Ok, I'm going to look up how to do the checkbox. Hopefully it's straightforward and doesn't require too much code. The race condition I had in mind was when you have two windows open and you change the toggle in one, and then again in the other. Stefano says that his Konq does the change live, but that doesn't seem to be working for me (not sure if my Konq, KF5 or Qt is to blame), but in any case if the change happens live, then that solves it and I guess there is no race condition. rrosch: Ok, I'm going to look up how to do the checkbox. Hopefully it's straightforward and doesn't… | |||||
OK, this isn't what I call a race condition (because it can be triggered very very slowly, that would be a snails race...). I call this a synchronization issue. At least konqueror is a unique service (single process) these days so it's easier than before, no DBus needed. If you have multiple windows open, I don't see how checking the action in one would immediately affect the other, you would have to implement that. dfaure: OK, this isn't what I call a race condition (because it can be triggered very very slowly, that… | |||||
579 | toggleShowHiddenFolders->setChecked(currentButtonInfo().showHiddenFolders); | ||||
580 | } | ||||
566 | buttonPopup->addSeparator(); | 581 | buttonPopup->addSeparator(); | ||
567 | buttonPopup->addAction(QIcon::fromTheme("edit-delete"), i18n("Remove"), this, SLOT(slotRemove())); | 582 | buttonPopup->addAction(QIcon::fromTheme("edit-delete"), i18n("Remove"), this, SLOT(slotRemove())); | ||
568 | buttonPopup->addSeparator(); | 583 | buttonPopup->addSeparator(); | ||
569 | buttonPopup->addMenu(m_menu); | 584 | buttonPopup->addMenu(m_menu); | ||
570 | buttonPopup->exec(QCursor::pos()); | 585 | buttonPopup->exec(QCursor::pos()); | ||
571 | delete buttonPopup; | 586 | delete buttonPopup; | ||
572 | } | 587 | } | ||
573 | return true; | 588 | return true; | ||
▲ Show 20 Lines • Show All 321 Lines • Show Last 20 Lines |
This should be an assert instead, you don't create the action if canToggle... is false.
Q_ASSERT(currentButtonInfo().canToggleShowHiddenFolders);