Bug 231366 - Shorten breadcrumb in file browser by dropping file icons
Summary: Shorten breadcrumb in file browser by dropping file icons
Status: RESOLVED FIXED
Alias: None
Product: amarok
Classification: Applications
Component: File Browser (show other bugs)
Version: 2.3.0
Platform: Arch Linux Linux
: NOR normal
Target Milestone: ---
Assignee: Amarok Developers
URL:
Keywords:
: 231367 (view as bug list)
Depends on:
Blocks:
 
Reported: 2010-03-20 01:27 UTC by Karthik Periagaram
Modified: 2010-06-10 13:17 UTC (History)
3 users (show)

See Also:
Latest Commit:
Version Fixed In: 2.3.2


Attachments
How it looks currently (7.20 KB, image/png)
2010-03-20 12:15 UTC, Karthik Periagaram
Details
Amarok breadcrumb resize problem (139.82 KB, image/jpeg)
2010-04-28 16:04 UTC, Terényi, Balázs
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Karthik Periagaram 2010-03-20 01:27:39 UTC
Version:           2.3.0 (using KDE 4.4.1)
OS:                Linux
Installed from:    Archlinux Packages

Currently, the breadcrumb atop the newly redesigned file browser in Amarok wastes a lot of space trying to show folder icons (which are not even the same as the custom icon set in the .directory files). As a result, there's no space left to show the important information.

A typical folder structure will look like this: /path/Music/artist/album/<songs>. The path takes up most of the space because of the icons. As a result, the artist and album directories have no space left to display. Currently,  the solution is to expand the pane containing the file browser. This is not an ideal solution.

My solution for this is to drop the icons. This will:

1) save up considerable space in the breadcrumb to show at least the last two directories.
2) relieve any confusion arising from the "Home" icon from Dolphin being used for the Music Sources Home. This used to be a problem in the old version of the file browser (one that's been arguably fixed by the current redesign), but it's still weird. The Music Sources icon isn't really the point of this bug, but fixing this will help with that problem - a nice added benefit.

If I'm not clear on anything, please let me know. Thanks!
Comment 1 Mikko C. 2010-03-20 09:12:40 UTC
*** Bug 231367 has been marked as a duplicate of this bug. ***
Comment 2 Karthik Periagaram 2010-03-20 12:15:29 UTC
Created attachment 41777 [details]
How it looks currently

The attachment shows how the breadcrumb currently adapts to navigation into my Music directory. Eventually, only the icons remain, adding little to no information about where the current directory is.

(Dolphin's breadcrumb handles this issue better by a) avoiding icons and b) collapsing long paths into the previous ">" symbol and showing only the current directory if space is limited. That would be the ideal solution to this bug.)

Sorry for the duplicate submission.
Comment 3 Terényi, Balázs 2010-04-28 11:17:50 UTC
I compiled the 2.3.1 beta to get a usable file browser. The breadcrumb is better than before, but I have still problems with it:
1. I have to use amarok maximized because of the breadcrumb
2. The breadcrumb still has the useless folder icons
3. The lineedit mode can't be set to be the default (I hate breadcrumbs)
4. The biggest problem is, that the breadcrumb forces to resize the Media Sources dock. I have now an about 70% wide file browser and a 10% wide Context and a 20% wide Playlist widget. It's useless without tabbing the docks.
5. My music files are in 15 folder deep on an nfs mount. At home and at work too. Please take this into account.

Thanks in advance,
Balázs
Comment 4 Karthik Periagaram 2010-04-28 13:45:45 UTC
(In reply to comment #3)

I think there's some misunderstanding here. I have the git version compiled on April 25 and the file browser shows only a portion or sometimes only the last directory. So, you no longer have to have Amarok full screened because of the file browser breadcrumb. The breadcrumb still has folder icons, but as I'll explain in a bit, they aren't much of a problem now as the breadcrumb shortens as you navigate deeper. It no longer forces you to resize your collection browser view. The only valid concern is that the lineedit mode is still not remembered between sessions.

There is however, one clarification I would like to add to the original bug. I would like to suggest that the preceding ">" should show a cascading path like Dolphin does, rather than other folders in the same directory. Let me explain this further...

As it seems to currently work, the file browser hides the preceding breadcrumb and renders something like,

(Local) > / > home > karthikp > Music > 65daysofstatic > The Destruction Of Small Ideas

as 

(Local) > The Destruction Of Small Ideas

This is great. It scales really well and even with the directory icons preceding each level name, the breadcrumb is really usable. Now, when I click on the ">", here's what Dolphin would show (pop up presented in glorious ASCII-vision(TM) ):

(Local) > The Destruction Of Small Ideas
       --------------------------------------------
      |  /                                         |
      |    home                                    |
      |      karthikp                              |
      |        Music                               |
      |          65daysofstatic                    |
      |            The Destruction Of Small Ideas  |
       --------------------------------------------

This lets me navigate up and down the breadcrumb even when the entire breadcrumb isn't visible. That's really nice. But, here's what Amarok shows:

(Local) > The Destruction Of Small Ideas
       --------------------------------------------
      |  One Time For All Time                     |
      |  The Destruction Of Small Ideas            |
      |  The Fall Of Math                          |
       --------------------------------------------

In other words, it shows the other directories at the same level. This is the behavior of the ">" when the entire breadcrumb is visible.

This is a recipe for disaster at the Music/ level. This causes the screen to be filled with hundreds of Artists. Hence, it would be better to change the behavior of the ">" to match Dolphin when the breadcrumb is collapsed. The primary function of the breadcrumb is to allow navigation between directory levels. When the entire breadcrumb isn't visible, the ">" needs to handle this function.

I hope this makes things clearer. As always, I'll gladly clarify further. Thanks for addressing this bug for the next release!
Comment 5 Terényi, Balázs 2010-04-28 16:04:33 UTC
Created attachment 43080 [details]
Amarok breadcrumb resize problem

Here You can see, that the file browser panel resizes itself bigger and bigger to this size. After this (I can't reproduce it exactly, but after some browsing) the file browser panel can not be resized anymore by the user (with the handle).
Simply clicking many times on the last folder in the breadcrumb makes the panel bigger and bigger. It's 2.3.1 beta.
Comment 6 Rick W. Chen 2010-06-10 12:58:17 UTC
commit b1ef82b2dabc1c2084facef9ea7451417c825c7c
Author: Rick W. Chen <stuffcorpse@archlinux.us>
Date:   Thu Jun 10 18:38:57 2010 +1200

    Shorten breadcrumb in file browser by dropping file icons
    
    BUG:231366

diff --git a/ChangeLog b/ChangeLog
index d4203dd..05a65a4 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -28,6 +28,7 @@ VERSION 2.3.2-Beta 1
       Patch by Richard Longland <rlongland@hotmail.com>.
 
   BUGFIXES:
+    * Drop file icons in file browser breadcrumbs. (BR 231366)
     * Fixed keyboard navigation in file browser. (BR 240668)
       Patch thanks to Hannes Koller.
     * Fixed dropping files to playlist from Konqueror. (BR 235722)
diff --git a/src/browsers/BrowserBreadcrumbItem.cpp b/src/browsers/BrowserBreadcrumbItem.cpp
index cb70b17..8befcee 100644
--- a/src/browsers/BrowserBreadcrumbItem.cpp
+++ b/src/browsers/BrowserBreadcrumbItem.cpp
@@ -126,7 +126,7 @@ BrowserBreadcrumbItem::BrowserBreadcrumbItem( const QString &name, const QString
         menu->setContentsMargins( offset, 1, 1, 2 );
     }
 
-    m_mainButton = new BreadcrumbItemButton( KIcon( "folder-amarok" ), name, this );
+    m_mainButton = new BreadcrumbItemButton( name, this );
     
     connect( m_mainButton, SIGNAL( sizePolicyChanged() ), this, SLOT( updateSizePolicy() ) );
 
diff --git a/src/browsers/filebrowser/FileBrowser.cpp b/src/browsers/filebrowser/FileBrowser.cpp
index 35ff68c..dfaa777 100644
--- a/src/browsers/filebrowser/FileBrowser.cpp
+++ b/src/browsers/filebrowser/FileBrowser.cpp
@@ -113,8 +113,6 @@ FileBrowser::FileBrowser( const char * name, QWidget *parent )
 
     setLongDescription( i18n( "The file browser lets you browse files anywhere on your system, regardless of whether these files are part of your local collection. You can then add these files to the playlist as well as perform basic file operations." ) );
     setImagePath( KStandardDirs::locate( "data", "amarok/images/hover_info_files.png" ) );
-
-    
 }
 
 FileBrowser::~FileBrowser()
diff --git a/src/widgets/BreadcrumbItemButton.cpp b/src/widgets/BreadcrumbItemButton.cpp
index 89bba9d..9a99b2a 100644
--- a/src/widgets/BreadcrumbItemButton.cpp
+++ b/src/widgets/BreadcrumbItemButton.cpp
@@ -41,6 +41,13 @@ BreadcrumbItemButton::BreadcrumbItemButton( QWidget *parent )
     init();
 }
 
+BreadcrumbItemButton::BreadcrumbItemButton( const QString &text, QWidget *parent )
+    : Amarok::ElidingButton( text, parent )
+    , m_displayHint( 0 )
+{
+    init();
+}
+
 BreadcrumbItemButton::BreadcrumbItemButton( const QIcon &icon, const QString &text, QWidget *parent )
     : Amarok::ElidingButton( icon, text, parent )
     , m_displayHint( 0 )
@@ -123,13 +130,21 @@ BreadcrumbItemButton::paintEvent( QPaintEvent* event )
     int left, top, right, bottom;
     getContentsMargins ( &left, &top, &right, &bottom );
     const int padding = 2;
-    const int iconWidth = iconSize().width();
-    const int iconHeight = iconSize().height();
-    const int iconTop = ( (buttonHeight - top - bottom) - iconHeight ) / 2;
-    const QRect iconRect( left + padding, iconTop, iconWidth, iconHeight );
-    painter.drawPixmap( iconRect, icon().pixmap( iconSize() ) );
+    int xoffset;
+
+    if( !icon().isNull() )
+    {
+        const int iconWidth = iconSize().width();
+        const int iconHeight = iconSize().height();
+        const int iconTop = ( (buttonHeight - top - bottom) - iconHeight ) / 2;
+        const QRect iconRect( left + padding, iconTop, iconWidth, iconHeight );
+        painter.drawPixmap( iconRect, icon().pixmap( iconSize() ) );
+        xoffset = left + (padding * 2) + iconWidth;
+    }
+    else
+        xoffset = left + (padding * 2);
 
-    const QRect textRect( left + (padding * 2) + iconWidth, top, buttonWidth, buttonHeight);
+    const QRect textRect( xoffset, top, buttonWidth, buttonHeight);
     painter.drawText(textRect, Qt::AlignVCenter, text());
 }
 
@@ -169,7 +184,10 @@ BreadcrumbItemButton::sizeHint() const
 {
     QSize size = Amarok::ElidingButton::sizeHint();
     QFontMetrics fm( font() );
-    size.setWidth( fm.width( text() ) + iconSize().width() + 8 );
+    int width = fm.width( text() );
+    if( !icon().isNull() )
+        width += iconSize().width();
+    size.setWidth( width + 8 );
     return size;
 }
 
diff --git a/src/widgets/BreadcrumbItemButton.h b/src/widgets/BreadcrumbItemButton.h
index 3b1d729..8bdf07a 100644
--- a/src/widgets/BreadcrumbItemButton.h
+++ b/src/widgets/BreadcrumbItemButton.h
@@ -39,6 +39,7 @@ class BreadcrumbItemButton : public Amarok::ElidingButton
 
     public:
         BreadcrumbItemButton( QWidget* parent );
+        BreadcrumbItemButton( const QString &text, QWidget *parent );
         BreadcrumbItemButton( const QIcon &icon, const QString &text, QWidget *parent );
         virtual ~BreadcrumbItemButton();
 
diff --git a/src/widgets/ElidingButton.cpp b/src/widgets/ElidingButton.cpp
index 63eaf2f..61b3581 100644
--- a/src/widgets/ElidingButton.cpp
+++ b/src/widgets/ElidingButton.cpp
@@ -50,7 +50,8 @@ ElidingButton::~ElidingButton()
 void ElidingButton::init()
 {
     m_isElided = false;
-    setMinimumWidth( iconSize().width() );
+    if( !icon().isNull() )
+        setMinimumWidth( iconSize().width() );
 }
 
 QSizePolicy ElidingButton::sizePolicy() const
@@ -87,7 +88,7 @@ void ElidingButton::setText( const QString &text )
 void ElidingButton::elideText( const QSize &widgetSize )
 {
     const int width = widgetSize.width();
-    const int iconWidth = iconSize().width();
+    const int iconWidth = icon().isNull() ? 0 : iconSize().width();
 
     int left, top, right, bottom;
     getContentsMargins( &left, &top, &right, &bottom );