[Differential] [Changed Subscribers] D3200: Add a widget gallery page to the Dashboard.

[davidedmundson (David Edmundson)]

davidedmundson added inline comments.

INLINE COMMENTS

> DashboardRepresentation.qml:101
> +    function updateWidgetExplorer() {
> +        if (tabBar.activeTab == 1 /* Widgets */ || tabBar.hoveredTab == 1) {
> +            root.widgetExplorer = widgetExplorerComponent.createObject(root);

I don't get why we are we creating this dynamically? and why on hover?

I assume it's because WE is costly?  and doing it on hover makes it seem faster?

but this code has a quirk that if you hover and then click (a fairly common pattern) you'll actually be creating this component twice.

A Loader with: active: activeTab == 1 || hoveredTab ==1 would be ideal and more declarative.

> DashboardRepresentation.qml:156
> +
> +            sourceModel: root.widgetExplorer ? root.widgetExplorer.filterModel : null
> +            filterCallback: function(row, value) {

It's better to just adjust Widget Explorer if we need to add functionality into WidgetExplorer

Doing something like this is the sort of thing that's going to unnoticeably break in the future when someone re-arranges those categories.

> DashboardTabBar.qml:30
> +
> +    onContainsMouseChanged: {
> +        if (containsMouse) {

for every mouse move you have two of these being emitted in sequence

one from the old tab, one from the new tab.

Can you be guarantee what order that happens in?

REPOSITORY
  rPLASMADESKTOP Plasma Desktop

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D3200

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: hein, #plasma, mart
Cc: davidedmundson, mart, plasma-devel, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20161101/e3204bf9/attachment-0001.html>