From 739be6eb6ce81aedf626b365b65fd999f89702cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaakko=20Ker=C3=A4nen?= Date: Sat, 23 Apr 2016 16:23:44 +0300 Subject: [PATCH] UI|Home: Usability improvements for the Packages dialog Selecting packages for a game is now more streamlined. Packages can be added and removed with a single mouse click. Clicking on a selected package will highlight it on the right-side package browser. --- .../include/ui/widgets/homeitemwidget.h | 1 + .../include/ui/widgets/homemenuwidget.h | 4 + .../include/ui/widgets/packageswidget.h | 13 ++ .../client/src/ui/dialogs/packagesdialog.cpp | 202 +++++------------- .../client/src/ui/widgets/homeitemwidget.cpp | 16 +- .../client/src/ui/widgets/homemenuwidget.cpp | 18 +- .../client/src/ui/widgets/packageswidget.cpp | 112 ++++++++-- 7 files changed, 192 insertions(+), 174 deletions(-) diff --git a/doomsday/apps/client/include/ui/widgets/homeitemwidget.h b/doomsday/apps/client/include/ui/widgets/homeitemwidget.h index d1f7e5fa2f..a3afbcd25a 100644 --- a/doomsday/apps/client/include/ui/widgets/homeitemwidget.h +++ b/doomsday/apps/client/include/ui/widgets/homeitemwidget.h @@ -43,6 +43,7 @@ class HomeItemWidget : public de::GuiWidget, public de::IAssetGroup de::LabelWidget &label(); void addButton(de::ButtonWidget *button); + void setKeepButtonsVisible(bool yes); virtual void setSelected(bool selected); bool isSelected() const; diff --git a/doomsday/apps/client/include/ui/widgets/homemenuwidget.h b/doomsday/apps/client/include/ui/widgets/homemenuwidget.h index 1e1b7f2e0a..be9fb02d6f 100644 --- a/doomsday/apps/client/include/ui/widgets/homemenuwidget.h +++ b/doomsday/apps/client/include/ui/widgets/homemenuwidget.h @@ -49,6 +49,10 @@ class HomeMenuWidget : public de::MenuWidget */ void setSelectedIndex(int index); +signals: + void selectedIndexChanged(int index); + void itemClicked(int index); + protected slots: void mouseActivityInItem(); void itemSelectionChanged(); diff --git a/doomsday/apps/client/include/ui/widgets/packageswidget.h b/doomsday/apps/client/include/ui/widgets/packageswidget.h index de2ebfa2d1..727ef3c189 100644 --- a/doomsday/apps/client/include/ui/widgets/packageswidget.h +++ b/doomsday/apps/client/include/ui/widgets/packageswidget.h @@ -55,11 +55,24 @@ class PackagesWidget : public de::GuiWidget, public de::IPersistent void setButtonHandler(IButtonHandler &buttonHandler); void setButtonLabels(de::String const &buttonLabel, de::String const &highlightedButtonLabel); + void setButtonImages(de::DotPath const &styleId, + de::DotPath const &highlightedStyleId); + void setActionButtonAlwaysShown(bool showActions); void setColorTheme(ColorTheme unselectedItem, ColorTheme selectedItem, ColorTheme loadedUnselectedItem, ColorTheme loadedSelectedItem); void populate(); + void updateItems(); + + /** + * Finds the item for a package, if it is currently listed. + * @param packageId Package identifier. + * @return Item. + */ + de::ui::Item const *itemForPackage(de::String const &packageId) const; + + void scrollToPackage(de::String const &packageId) const; de::LineEditWidget &searchTermsEditor(); diff --git a/doomsday/apps/client/src/ui/dialogs/packagesdialog.cpp b/doomsday/apps/client/src/ui/dialogs/packagesdialog.cpp index 73cc4351ae..0ab3067e10 100644 --- a/doomsday/apps/client/src/ui/dialogs/packagesdialog.cpp +++ b/doomsday/apps/client/src/ui/dialogs/packagesdialog.cpp @@ -45,6 +45,7 @@ DENG_GUI_PIMPL(PackagesDialog) , public ChildWidgetOrganizer::IWidgetFactory , public PackagesWidget::IPackageStatus , public PackagesWidget::IButtonHandler +, DENG2_OBSERVES(Widget, ChildAddition) { StringList requiredPackages; // loaded first, cannot be changed StringList selectedPackages; @@ -89,13 +90,6 @@ DENG_GUI_PIMPL(PackagesDialog) return _file; } - /*void setFile(File const &packFile) - { - file = &packFile; - info = &file->objectNamespace().subrecord(Package::VAR_PACKAGE); - notifyChange(); - }*/ - private: File const *_file = nullptr; Record const *_info = nullptr; @@ -108,26 +102,24 @@ DENG_GUI_PIMPL(PackagesDialog) class SelectedPackageWidget : public HomeItemWidget { public: - SelectedPackageWidget(SelectedPackageItem const &item) - : _item(&item) + SelectedPackageWidget(SelectedPackageItem const &item, + PackagesDialog &owner) + : _owner(owner) + , _item(&item) { - useColorTheme(Normal, Inverted); + useColorTheme(Normal, Normal); - _infoButton = new PopupButtonWidget; - _infoButton->setText(tr("...")); - _infoButton->setFont("small"); - _infoButton->margins().setTopBottom("unit"); - _infoButton->setPopup([this] (PopupButtonWidget const &) + _removeButton = new ButtonWidget; + _removeButton->setImage(new StyleProceduralImage("close.ringless", *_removeButton)); + _removeButton->setOverrideImageSize(style().fonts().font("small").height().value()); + _removeButton->margins().setTopBottom("unit"); + _removeButton->setActionFn([this] () { - auto *pop = new PopupMenuWidget; - pop->setColorTheme(Inverted); - pop->items() << new ui::SubwidgetItem( - tr("Info"), ui::Down, - [this] () -> PopupWidget * { return makeInfoPopup(); }); - return pop; - } - , ui::Down); - addButton(_infoButton); + _owner.d->removePackage(packageId()); + _owner.d->browser->updateItems(); + }); + addButton(_removeButton); + setKeepButtonsVisible(true); // Package icon. icon().set(Background()); @@ -137,90 +129,8 @@ DENG_GUI_PIMPL(PackagesDialog) Rule const &height = style().fonts().font("default").height(); icon().setOverrideImageSize(height.value()); icon().rule().setInput(Rule::Width, height + rule("dialog.gap")*2); - - /* - add(_title = new LabelWidget); - _title->setSizePolicy(ui::Fixed, ui::Expand); - _title->setAlignment(ui::AlignLeft); - _title->setTextLineAlignment(ui::AlignLeft); - _title->margins().setBottom("").setLeft("unit"); - - add(_subtitle = new LabelWidget); - _subtitle->setSizePolicy(ui::Fixed, ui::Expand); - _subtitle->setAlignment(ui::AlignLeft); - _subtitle->setTextLineAlignment(ui::AlignLeft); - _subtitle->setFont("small"); - _subtitle->setTextColor("accent"); - _subtitle->margins().setTop("").setBottom("unit").setLeft("unit"); - - add(_loadButton = new ButtonWidget); - _loadButton->setSizePolicy(ui::Expand, ui::Expand); - _loadButton->setAction(new LoadAction(*this)); - - add(_infoButton = new PopupButtonWidget); - _infoButton->setSizePolicy(ui::Expand, ui::Fixed); - _infoButton->setText(_E(s)_E(B) + tr("...")); - _infoButton->setPopup([this] (PopupButtonWidget const &) { - return makeInfoPopup(); - }, ui::Left); - - createTagButtons(); - - AutoRef titleWidth(rule().width() - - _loadButton->rule().width() - - _infoButton->rule().width()); - - _title->rule() - .setInput(Rule::Width, titleWidth) - .setInput(Rule::Left, rule().left()) - .setInput(Rule::Top, rule().top()); - _subtitle->rule() - .setInput(Rule::Width, titleWidth) - .setInput(Rule::Left, rule().left()) - .setInput(Rule::Top, _title->rule().bottom()); - - _loadButton->rule() - .setInput(Rule::Right, rule().right() - _infoButton->rule().width()) - .setMidAnchorY(rule().midY()); - _infoButton->rule() - .setInput(Rule::Right, rule().right()) - .setInput(Rule::Height, _loadButton->rule().height()) - .setMidAnchorY(rule().midY()); - - rule().setInput(Rule::Width, rule("dialog.packages.width")) - .setInput(Rule::Height, _title->rule().height() + - _subtitle->rule().height() + _tags.at(0)->rule().height()); - */ - } -/* - void createTagButtons() - { - SequentialLayout layout(_subtitle->rule().left(), - _subtitle->rule().bottom(), ui::Right); - - for(QString tag : Package::tags(*_item->file)) - { - auto *btn = new ButtonWidget; - btn->setText(_E(l) + tag.toLower()); - updateTagButtonStyle(btn, "accent"); - btn->setSizePolicy(ui::Expand, ui::Expand); - btn->margins() - .setTop("unit").setBottom("unit") - .setLeft("gap").setRight("gap"); - add(btn); - - layout << *btn; - _tags.append(btn); - } } - void updateTagButtonStyle(ButtonWidget *tag, String const &color) - { - tag->setFont("small"); - tag->setTextColor(color); - tag->set(Background(Background::Rounded, style().colors().colorf(color), 6)); - }*/ - void updateContents() { if(_item->info()) @@ -231,33 +141,6 @@ DENG_GUI_PIMPL(PackagesDialog) { label().setText(_item->packageId()); } - - /* - _subtitle->setText(packageId()); - - String auxColor = "accent"; - - if(isLoaded()) - { - _loadButton->setText(tr("Unload")); - _loadButton->setTextColor("altaccent"); - _loadButton->setBorderColor("altaccent"); - _title->setFont("choice.selected"); - auxColor = "altaccent"; - } - else - { - _loadButton->setText(tr("Load")); - _loadButton->setTextColor("text"); - _loadButton->setBorderColor("text"); - _title->setFont("default"); - } - - _subtitle->setTextColor(auxColor); - for(ButtonWidget *b : _tags) - { - updateTagButtonStyle(b, auxColor); - }*/ } String packageId() const @@ -271,13 +154,9 @@ DENG_GUI_PIMPL(PackagesDialog) } private: + PackagesDialog &_owner; SelectedPackageItem const *_item; - //LabelWidget *_title; - //LabelWidget *_subtitle; - //QList _tags; - PopupButtonWidget *_infoButton; - //ButtonWidget *_loadButton; - //PopupButtonWidget *_infoButton; + ButtonWidget *_removeButton; }; Instance(Public *i) : Base(i) @@ -318,13 +197,27 @@ DENG_GUI_PIMPL(PackagesDialog) .setInput(Rule::Top, gameTitle->rule().bottom()) .setInput(Rule::Width, rule("dialog.packages.width")); menu->organizer().setWidgetFactory(*this); + menu->audienceForChildAddition() += this; self.leftArea().enableIndicatorDraw(true); + QObject::connect(menu, &HomeMenuWidget::itemClicked, [this] (int index) + { + // Update the remove button position. + if(index >= 0) + { + browser->scrollToPackage(menu->items().at(index) + .as().packageId()); + } + }); + // Package browser. self.rightArea().add(browser = new PackagesWidget); + browser->setActionButtonAlwaysShown(true); browser->setPackageStatus(*this); browser->setButtonHandler(*this); - browser->setButtonLabels(tr("Add"), tr("Remove")); + //browser->setButtonLabels(tr("Add"), tr("Remove")); + browser->setButtonLabels("", ""); + browser->setButtonImages("create", "close.ringless"); //browser->setColorTheme(Normal, Normal, Normal, Normal); browser->rule() .setInput(Rule::Left, self.rightArea().contentRule().left()) @@ -333,6 +226,11 @@ DENG_GUI_PIMPL(PackagesDialog) self.rightArea().enableIndicatorDraw(true); } + ~Instance() + { + menu->audienceForChildAddition() -= this; + } + void populate() { menu->items().clear(); @@ -380,7 +278,7 @@ DENG_GUI_PIMPL(PackagesDialog) GuiWidget *makeItemWidget(ui::Item const &item, GuiWidget const *) { - return new SelectedPackageWidget(item.as()); + return new SelectedPackageWidget(item.as(), self); } void updateItemWidget(GuiWidget &widget, ui::Item const &) @@ -402,14 +300,25 @@ DENG_GUI_PIMPL(PackagesDialog) } else { - selectedPackages.removeOne(packageId); - auto pos = menu->items().findData(packageId); - DENG2_ASSERT(pos != ui::Data::InvalidPos); - menu->items().remove(pos); + removePackage(packageId); } updateNothingIndicator(); } + + void removePackage(String const &packageId) + { + selectedPackages.removeOne(packageId); + auto pos = menu->items().findData(packageId); + DENG2_ASSERT(pos != ui::Data::InvalidPos); + menu->items().remove(pos); + } + + void widgetChildAdded(Widget &child) + { + menu->setSelectedIndex(menu->items().find( + *menu->organizer().findItemForWidget(child.as()))); + } }; PackagesDialog::PackagesDialog(String const &titleText) @@ -426,7 +335,8 @@ PackagesDialog::PackagesDialog(String const &titleText) } heading().setImage(style().images().image("package")); buttons() - << new DialogButtonItem(Default | Accept, tr("Close")) + << new DialogButtonItem(Default | Accept, tr("OK")) + << new DialogButtonItem(Reject, tr("Cancel")) << new DialogButtonItem(Action, style().images().image("refresh"), new SignalAction(this, SLOT(refreshPackages()))); diff --git a/doomsday/apps/client/src/ui/widgets/homeitemwidget.cpp b/doomsday/apps/client/src/ui/widgets/homeitemwidget.cpp index c49fed7f4f..814db8a3ee 100644 --- a/doomsday/apps/client/src/ui/widgets/homeitemwidget.cpp +++ b/doomsday/apps/client/src/ui/widgets/homeitemwidget.cpp @@ -96,6 +96,7 @@ DENG_GUI_PIMPL(HomeItemWidget) AnimationRule *labelRightMargin; Rule const *buttonsWidth = nullptr; bool selected = false; + bool keepButtonsVisible = false; DotPath bgColor { "transparent" }; DotPath selectedBgColor { "background" }; DotPath textColor { "text" }; @@ -242,7 +243,7 @@ void HomeItemWidget::setSelected(bool selected) { d->showButtons(true); } - else + else if(!d->keepButtonsVisible) { d->showButtons(false); } @@ -370,3 +371,16 @@ void HomeItemWidget::addButton(ButtonWidget *button) button->hide(); d->updateButtonLayout(); } + +void HomeItemWidget::setKeepButtonsVisible(bool yes) +{ + d->keepButtonsVisible = yes; + if(yes) + { + d->showButtons(true); + } + else if(!d->selected) + { + d->showButtons(false); + } +} diff --git a/doomsday/apps/client/src/ui/widgets/homemenuwidget.cpp b/doomsday/apps/client/src/ui/widgets/homemenuwidget.cpp index 6444de3bbc..292e0cff85 100644 --- a/doomsday/apps/client/src/ui/widgets/homemenuwidget.cpp +++ b/doomsday/apps/client/src/ui/widgets/homemenuwidget.cpp @@ -123,10 +123,12 @@ void HomeMenuWidget::setSelectedIndex(int index) // If not, we are observing the asset and will scroll when it is ready. if(assets().isReady()) { + qDebug() << "immediate scroll"; d->scrollToSelected(); } else { + qDebug() << "deferred scroll"; assets().audienceForStateChange() += d; } } @@ -135,8 +137,10 @@ void HomeMenuWidget::setSelectedIndex(int index) void HomeMenuWidget::mouseActivityInItem() { - /*auto *clickedItem = dynamic_cast(sender()); + auto *clickedItem = dynamic_cast(sender()); + emit itemClicked(childWidgets().indexOf(clickedItem)); +/* // Radio button behavior: other items will be deselected. for(int i = 0; i < childWidgets().size(); ++i) { @@ -161,13 +165,19 @@ void HomeMenuWidget::itemSelectionChanged() { if(d->selectedIndex >= 0) { - // Deselect the previous selection. - if(auto *item = childWidgets().at(d->selectedIndex)->maybeAs()) + auto children = childWidgets(); + if(d->selectedIndex < children.size()) { - item->setSelected(false); + // Deselect the previous selection. + if(auto *item = children.at(d->selectedIndex)->maybeAs()) + { + item->setSelected(false); + } } } d->selectedIndex = d->previousSelectedIndex = newSelection; + + emit selectedIndexChanged(d->selectedIndex); } } } diff --git a/doomsday/apps/client/src/ui/widgets/packageswidget.cpp b/doomsday/apps/client/src/ui/widgets/packageswidget.cpp index 659b59815b..bb93e9d4db 100644 --- a/doomsday/apps/client/src/ui/widgets/packageswidget.cpp +++ b/doomsday/apps/client/src/ui/widgets/packageswidget.cpp @@ -87,8 +87,10 @@ DENG_GUI_PIMPL(PackagesWidget) ButtonWidget *clearSearch; HomeMenuWidget *menu; String buttonLabels[2]; + DotPath buttonImages[2]; QStringList filterTerms; bool showHidden = false; + bool actionOnlyForSelection = true; IPackageStatus const *packageStatus = &isPackageLoaded; IButtonHandler *buttonHandler = &loadOrUnloadPackage; @@ -119,6 +121,11 @@ DENG_GUI_PIMPL(PackagesWidget) info = &file->objectNamespace().subrecord(Package::VAR_PACKAGE); notifyChange(); } + + void update() + { + notifyChange(); + } }; /** @@ -140,16 +147,18 @@ DENG_GUI_PIMPL(PackagesWidget) icon().setOverrideImageSize(height.value()); icon().rule().setInput(Rule::Width, height + rule("gap")*2); - _loadButton = new ButtonWidget; - _loadButton->setActionFn([this] () + _actionButton = new ButtonWidget; + _actionButton->setOverrideImageSize(style().fonts().font("default").height().value()); + _actionButton->setActionFn([this] () { - _owner.d->buttonHandler->packageButtonClicked(*_loadButton, packageId()); + _owner.d->buttonHandler->packageButtonClicked(*_actionButton, packageId()); updateContents(); }); connect(this, &HomeItemWidget::doubleClicked, [this] () { - _loadButton->trigger(); + _actionButton->trigger(); }); - addButton(_loadButton); + addButton(_actionButton); + setKeepButtonsVisible(!_owner.d->actionOnlyForSelection); /*add(_title = new LabelWidget); _title->setSizePolicy(ui::Fixed, ui::Expand); @@ -255,18 +264,30 @@ DENG_GUI_PIMPL(PackagesWidget) String auxColor = "accent"; + auto &imageIds = _owner.d->buttonImages; + if(_owner.d->packageStatus->isPackageHighlighted(packageId())) { - _loadButton->setText(_owner.d->buttonLabels[1]); - _loadButton->setColorTheme(invertColorTheme(_owner.d->loadedSelectedItem)); + _actionButton->setText(_owner.d->buttonLabels[1]); + if(!imageIds[1].isEmpty()) + { + _actionButton->setImage(new StyleProceduralImage(imageIds[1], *this)); + _actionButton->setImageColor(style().colors().colorf("text")); + } + _actionButton->setColorTheme(invertColorTheme(_owner.d->loadedSelectedItem)); icon().setImageColor(style().colors().colorf("accent")); useColorTheme(_owner.d->loadedUnselectedItem, _owner.d->loadedSelectedItem); auxColor = "background"; } else { - _loadButton->setText(_owner.d->buttonLabels[0]); - _loadButton->setColorTheme(invertColorTheme(_owner.d->selectedItem)); + _actionButton->setText(_owner.d->buttonLabels[0]); + if(!imageIds[0].isEmpty()) + { + _actionButton->setImage(new StyleProceduralImage(imageIds[0], *this)); + _actionButton->setImageColor(style().colors().colorf("inverted.text")); + } + _actionButton->setColorTheme(invertColorTheme(_owner.d->selectedItem)); icon().setImageColor(style().colors().colorf("text")); useColorTheme(_owner.d->unselectedItem, _owner.d->selectedItem); } @@ -293,7 +314,7 @@ DENG_GUI_PIMPL(PackagesWidget) //LabelWidget *_title; //LabelWidget *_subtitle; QList _tags; - ButtonWidget *_loadButton; + ButtonWidget *_actionButton; //PopupButtonWidget *_infoButton; }; @@ -377,6 +398,15 @@ DENG_GUI_PIMPL(PackagesWidget) } } + void updateItems() + { + menu->items().forAll([this] (ui::Item &item) + { + item.as().update(); + return LoopContinue; + }); + } + void updateFilterTerms() { /// @todo Parse quoted terms. -jk @@ -451,19 +481,7 @@ PackagesWidget::PackagesWidget(String const &name) : GuiWidget(name) , d(new Instance(this)) { - /*buttons() - << new DialogButtonItem(Default | Accept, tr("Close")) - << new DialogButtonItem(Action, style().images().image("refresh"), - new CallbackAction([this] () - { - App::fileSystem().refresh(); - d->populate(); - }));*/ - - //area().setContentSize(d->menu->rule().width(), d->menu->rule().height()); - rule().setInput(Rule::Height, - d->search->rule().height() + - d->menu->rule().height()); + rule().setInput(Rule::Height, d->search->rule().height() + d->menu->rule().height()); refreshPackages(); } @@ -486,6 +504,28 @@ void PackagesWidget::setButtonLabels(String const &buttonLabel, String const &hi d->populate(); } +void PackagesWidget::setButtonImages(DotPath const &styleId, DotPath const &highlightedStyleId) +{ + d->buttonImages[0] = styleId; + d->buttonImages[1] = highlightedStyleId; + + d->populate(); +} + +void PackagesWidget::setActionButtonAlwaysShown(bool showActions) +{ + d->actionOnlyForSelection = !showActions; + + // Update existing widgets. + for(auto *w : d->menu->childWidgets()) + { + if(HomeItemWidget *item = w->maybeAs()) + { + item->setKeepButtonsVisible(showActions); + } + } +} + void PackagesWidget::setColorTheme(ColorTheme unselectedItem, ColorTheme selectedItem, ColorTheme loadedUnselectedItem, ColorTheme loadedSelectedItem) { @@ -502,6 +542,32 @@ void PackagesWidget::populate() d->populate(); } +void PackagesWidget::updateItems() +{ + d->updateItems(); +} + +ui::Item const *PackagesWidget::itemForPackage(String const &packageId) const +{ + ui::DataPos found = d->menu->items().findData(packageId); + if(found != ui::Data::InvalidPos) + { + return &d->menu->items().at(found); + } + return nullptr; +} + +void PackagesWidget::scrollToPackage(String const &packageId) const +{ + if(auto const *item = itemForPackage(packageId)) + { + if(auto const *widget = d->menu->organizer().itemWidget(*item)) + { + d->menu->findTopmostScrollable().scrollToWidget(*widget); + } + } +} + LineEditWidget &PackagesWidget::searchTermsEditor() { return *d->search;