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
