[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-panel-devel
Subject: Re: Review Request: big revamp of Device Notifier
From: "Aaron Seigo" <aseigo () kde ! org>
Date: 2009-09-13 18:55:34
Message-ID: 20090913185534.19605.2473 () localhost
[Download RAW message or body]
> On 2009-09-11 19:44:22, Aaron Seigo wrote:
> > is the latest version the most recent available version of the patch? because it \
> > doesn't apply cleanly to trunk.
> > anyways ... i really don't like the "how long to show the popup" configuration \
> > item and the other 2 config items do need seem linguistic and layout adjustments, \
> > but that's easy to do once this is in. "how long" could remain configurable in \
> > the config file, but i don't think it belongs in the UI; that's just an excuse \
> > not to make a good default.
> > having to click twice for items with one action is not great. instead, when there \
> > is just one item how about just putting the action description as the sub-title \
> > text for the main item and launch it on click/select? so it would be:
> > ICON Camera
> > ICON Open with Digikam...
> >
> > in the case of multiple items, it could look like:
> >
> > ICON USB Storage Device
> > ICON 2 available actions ...
> >
> > and when it's clicked then the options show up. it could happen on hover, but i'm \
> > not sure that makes sense in this case.
> > if the options could also be made smaller that would also help show that these \
> > are "detail items that belong to the item above" better IMO; the icon should \
> > probably only need to be roughly as tall as the line of text?
> >
>
> Giulio Camuffo wrote:
> The problem is that the subtitle could be covered by the KCapacityBar. So we need \
> to find a solution, as to show the actions on hover.
> Aaron Seigo wrote:
> the capacity bar should be painted below any text.
>
> Giulio Camuffo wrote:
> that would solve all, but personally i dislike very much from an aesthetically pov \
> the items as they are painted in the current device notifier, with the increased \
> height. i find them fat. and if the volumes are umounted there would be no capacity \
> bar, and that would result in a lot of wasted white space
> Jacopo De Simoi wrote:
> Yeah, no unnececessary white space would be definitely a +
why would the items be taller if there is no capacity bar? the items should be as \
tall as necessary to show the content they have to show. no more, no less. and items \
do not need to be the same height (so if there is no capacity bar, that item may be \
shorter than others). there should be no unused whitepsace.
keeping in mind that the entire point of the device notifier is to give quick access \
to actions, i think it's just fine to have a bit of extra space for text saying what \
will happen when you click on the device icon.
another possibility might be to show the capacity bar under the icon, so that the top \
of the icon and the bottom of the capcity bar line up with the top and bottom of the \
text to the right.
> On 2009-09-11 19:44:22, Aaron Seigo wrote:
> > /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/devicenotifier.cpp, \
> > line 366 <http://reviewboard.kde.org/r/1370/diff/5/?file=10962#file10962line366>
> >
> > this is only needed if m_showOnlyRemovable changes, correct?
>
> wrote:
> it is used also to hide or show the devices when you (un)check the option "show all \
> the items" in the context menu.
> wrote:
> yes, that's what i said :)
>
> wrote:
> no, i mean when you right click the dialog, and you set the option to show or not \
> even devices that normally would be hidden too. like when you right click on the \
> Places panel in dolphin. In fact resetDevices() is called in configAccepted() (if \
> m_showOnlyRemovable changes) and in setAllItemsShown(bool shown) (if m_showAll \
> changes).
ok, let's back up a bit here.
that call to resetDevices is in DeviceNotifier::configAccepted. so i'm talking about \
when the configuration is changed. that means that in configAccepted, which is what \
line 365 here is, resetDevices _does not need to be called_ unless \
m_showOnlyRemovable changes. make sense?
now forward again :)
looking a m_showAll in NotifierView, Show All devices is always in the context menu \
and always enabled, even when there is nothing to show. probably, if there are no \
items to show there doesn't need to be a context menu in the view at all.
i'm still not really sure what the real world use cases for hiding/showing individual \
items in the notifier are other than "it's neat that i can"; i'm trying to imagine \
someone who has so many items listed there that it's unmanagable otherwise. or \
someone who keeps two items in one notifier widget and has a second notifier widget \
with some others? hm.
on the other hand, i can see this resulting in confusion when when someone plugs a \
device in, marks it to not be shown, then later plugs it in (and hopefully it's \
impossible to have different devices with the same name?) and it doesn't show up. \
particularly is days or weeks have passed between those two events.
and why would i NOT want my camera to show up when i plug it in? what's the use case \
for that?
this is why i'm still really unsure that turning the device *notifier* into a device \
*listing manager* in this way is the best of ideas.
there's really two similar ideas here it seems: managing devices that are plugged and \
unplugged (which may or may not be storage devices) and listing storage devices. we \
actually have a widget for both of those tasks, but the storage device lister doesn't \
provide access to actions related to the device, which the device notifier does.
this really comes back to use cases. can you provide use cases to go with the \
hide/show individual items feature?
oh, and the widget should "emit configNeedsSaving()" after calls to writeEntry :)
- Aaron
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/1370/#review2298
-----------------------------------------------------------
On 2009-09-11 21:32:17, Giulio Camuffo wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/1370/
> -----------------------------------------------------------
>
> (Updated 2009-09-11 21:32:17)
>
>
> Review request for Plasma.
>
>
> Summary
> -------
>
> This is a patch that modifies quite heavily the behaviour of the Device Notifier.
> It comes from here: \
> http://kde-look.org/content/show.php/Device+Manager?content=106051 It can show the \
> not removable devices too, it can mount them automatically or with a click, since \
> the "eject" button is a "mount" button when the volume is umounted. So that guy on \
> the dot will be ok. It can hide some items in the same way as Dolphin's places \
> (hide item/ show all). Finally, it shows the various opening actions under the \
> device instead of calling that xp-ish window.
>
> Diffs
> -----
>
> /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/notifierdialog.h 1022457 \
>
> /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/devicespaceinfodelegate.cpp \
> 1022457
> /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/devicespaceinfodelegate.h \
> 1022457
> /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/devicenotifier.cpp \
> 1022457
> /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/CMakeLists.txt 1022457
> /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/configurationpage.ui \
> PRE-CREATION
> /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/devicenotifier.h 1022457 \
>
> /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/notifierdialog.cpp \
> 1022457
> /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/notifierview.h 1022457
> /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/notifierview.cpp 1022457 \
>
> Diff: http://reviewboard.kde.org/r/1370/diff
>
>
> Testing
> -------
>
> I'm using it every day since I released 0.1 on Kde-look. I tried all the options on \
> my pc and they work. Some people on kde-look posted some comments about some \
> problems, but it seems to me they are very particular cases, so in my opinion it is \
> quite stable to go in trunk, but anyway review it! :)
>
> Screenshots
> -----------
>
> screen
> http://reviewboard.kde.org/r/1370/s/183/
>
>
> Thanks,
>
> Giulio
>
>
_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic