[Aptitude-devel] 002-qt-stubs-review round 2 part 1

[Daniel Burrows]

On Mon, Jul 19, 2010 at 10:50:20PM +0200, Piotr Galiszewski <piotr at galiszewski.pl> was heard to say:
> 2010/7/19 Daniel Burrows <dburrows at debian.org>:
> >> +     if(window->isMinimized())
> >> +       window->showNormal(); // unminimize
> >> +     window->raise();
> >> +     window->activateWindow();
> >
> >  Normally the window should be unminimized (since the user is
> > interacting with it) -- if we show a tab because of some background
> > process, I think it would be better to display a notification instead
> > of popping the window to the foreground.  Also, if the user's window
> > manager is configured to not raise on click, we're overriding their
> > preferences here.
> >
> >  In summary: is there a good reason to bring ourselves into the
> > foreground whenever a tab is displayed?  It seems impolite to me.
> >
> 
> I have made an assumption, that tabs are only opened on user request.
> Background processes shouldn't open new tabs. If they want to inform
> user about something, they would use status_widget (now
> active_background_jobs model) and add to widget "Show Details" button,
> which will open new tab
> I thought that if user decides to show some tabs, he want to use them
> immediately. Of course I could be wrong and this code could be easy
> fixed ;)

  I guess I look at it like this: if aptitude is the currently focused
window, then the user is already interacting with us.  If we are not
already the topmost window, then presumably they have configured this
explicitly for a reason and we should not second-guess them.

  On the other hand, if aptitude is NOT the currently focused window,
then either we have some sort of background event (which, as you noted,
won't happen right now), or a tab was somehow opened in a background
window -- for instance, the user asked to open a new tab and then
immediately minimized the window.  In the first case, it's far better
to display a notification if we need the user's attention; in the second
case, the user *explicitly* told us to go away, and coming back is
obnoxious.

  I should add that programs which refuse to stay minimized are a pet
peeve of mine.  Particularly when I switch to another workspace while
I'm waiting for something to deswap, then get forcibly context-switched
back.  (if it weren't for the fact that it's occasionally actually
appropriate, I'd just try to configure my WM to ignore those events)

> >> +      void tabs_manager::tabs_manager_impl::tab_deletion_requested(tab *tab_to_delete, bool force)
> >> +      {
> >> +     // when for example performing changes tab is closed, we do not destroy it but only hide
> >> +     if(tab_to_delete->get_policy() == tab::closing_policy_hide)
> >> +     {
> >> +       if(force)
> >
> >  This logic is a little hairy.
> >
> >  I almost would prefer a different signal/slot pair in this case, or
> > maybe connecting to the destroyed signal on the tab itself when we
> > attach it (in create_or_display<>).

  To be clear, what I was really referring to here is the way that the
"please close this tab" and "this tab is closing, hide it!" signals
go to the same place and have to be disambiguated via the "force"
argument.

> OK. I said that this code is tricky ;) I thought about his and decided
> that  it would be better to avoid series of objects cast in order to
> decide which variable should be cleared. Should I create new method
> for each tab_type (it is possible that there will be more types in the
> future)

  For now, I think you don't need to create a new method for every tab
type.  It might be something to consider in the future, but I think it
would probably just add more architecture without much gain.  If this
method starts to get long, this question should be revisited.

  Daniel