D14442: Unify window and tab title

David Hallas noreply at phabricator.kde.org
Sat Jul 28 19:07:51 BST 2018


hallas added a comment.


  In D14442#299644 <https://phabricator.kde.org/D14442#299644>, @markg wrote:
  
  > I think the logic you've chosen is wrong.
  >  Lets take this fruit store analogy.  You have a fruit store with an owner and fruit.
  >  The fruit is dumb and can't do anything outside itself (this would be a tab in dolphin).
  >  The owner (dolphinmainwindow) can look at fruit en change whatever it wants to change in the store (whole of dolphin).
  >
  > What you did is make the fruit able to talk to the owner...
  >
  > I think you should change this logic to add a "getCaption" to the tab logic. Then use m_activeViewContainer (i think you need to use that one) and ask the caption from the container. And call that function on each active container change.
  >  That would be a sound logic in my book and keeps responsibility where it should be.
  >  This would also prevent weird code constructs as you have now:
  >
  >   QString name = m_mainWindow->getCaption(url, currentTabPage() ? currentTabPage()->activeViewContainer() : nullptr);
  >   
  >
  > which would become:
  >
  >   QString name = m_activeViewContainer->getCaption();
  
  
  Thanks for the feedback, and nice analogy :)
  
  I tend to agree with you, when doing this change I was actually looking to place this code in a utility class, but I couldn't really find any other utility classes, so I ended up keeping the code as close to the original place as possible. But I think you are right that it doesn't really belong in the DolphinMainWindow class, so I will go ahead on fix up the commit.

REPOSITORY
  R318 Dolphin

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

To: hallas, #dolphin, ngraham, elvisangelaccio, markg
Cc: markg, kfm-devel, spoorun, navarromorales, firef, andrebarros, emmanuelp
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20180728/2dedb84c/attachment.htm>


More information about the kfm-devel mailing list