Add navigation history to forward/back buttons
Needs ReviewPublic

Authored by hallas on Feb 25 2019, 5:44 PM.

Details

Reviewers
ngraham
elvisangelaccio
Group Reviewers
Dolphin
VDG
Summary

Adds navigation history to forward/back buttons in the toolbar. This
changes the forward/back buttons in the toolbar to use the
KToolBarPopupAction class which provides access to a drop down menu.
This menu is then filled with up to 10 of the last hostory entries.

Test Plan

Browse some folders
Click the back drop down menu and navigate somewhere
Click the forward drop down menu and navigate somewhere

FEATURE: 157819

Diff Detail

Repository
R318 Dolphin
Branch
ktoolbar_popup_action (branched from master)
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 9586
Build 9604: arc lint + arc unit
hallas created this revision.Feb 25 2019, 5:44 PM
Restricted Application added a project: Dolphin. · View Herald TranscriptFeb 25 2019, 5:44 PM
Restricted Application added a subscriber: kfm-devel. · View Herald Transcript
hallas requested review of this revision.Feb 25 2019, 5:44 PM

This change is still WIP and is meant as a discussion point. I would like to get some feedback on this in general but most specifically around what to show in the navigation drop down? Currently I just put the full URL there but that is not very user friendly.

src/dolphinmainwindow.cpp
89

Should this be user configurable? Is 10 a good number?

627

What should we use as the action title? The full URL doesn't seem optimal :)

648

What should we use as the action title? The full URL doesn't seem optimal :)

1249

I copied some of this stuff from the implementation of KStandardAction::back but that function actually changes the icon on Right To Left systems, so should I do the same here?

1284

I copied some of this stuff from the implementation of KStandardAction::forward but that function actually changes the icon on Right To Left systems, so should I do the same here?

ngraham added a reviewer: VDG.Feb 25 2019, 5:53 PM
ngraham added inline comments.
src/dolphinmainwindow.cpp
89

Why does there have to be a maximum? If the user's done a lot of navigation, and they actually use the menu, they'll want to see everything.

ngraham requested changes to this revision.Feb 25 2019, 5:55 PM

UI-wise, I'm not thrilled by both buttons getting dropdown arrows to the right:

While it's undoubtedly more functional, it just looks... bad. :( And I'm not sure that the history menu feature is so important that it merits such an in-your-face UI. What most web browsers do these days is only show the history menu when you press-and-hold the back or forward button. What do you think about doing that instead? With the Breeze theme at least, when you give a ToolButton a menu, a tiny little dropdown arrow will appear in the corner of the button to alert the user that there's additional functionality there, which is much less visually obtrusive.

As for what to show in the menu, I would recommend removing the schema for local files, i.e. don't show file:// at the beginning.

This revision now requires changes to proceed.Feb 25 2019, 5:55 PM
hallas updated this revision to Diff 52595.Feb 26 2019, 10:34 AM

Disable delayed mode for the back/forward menus.
Increase maximum number of navigation entries to 100 (which is the same limit as the URL navigator).
Remove the schema from the URL for local URLs.

UI-wise, I'm not thrilled by both buttons getting dropdown arrows to the right:

While it's undoubtedly more functional, it just looks... bad. :( And I'm not sure that the history menu feature is so important that it merits such an in-your-face UI. What most web browsers do these days is only show the history menu when you press-and-hold the back or forward button. What do you think about doing that instead? With the Breeze theme at least, when you give a ToolButton a menu, a tiny little dropdown arrow will appear in the corner of the button to alert the user that there's additional functionality there, which is much less visually obtrusive.

As for what to show in the menu, I would recommend removing the schema for local files, i.e. don't show file:// at the beginning.

Thanks for the quick feedback :)

I have changed the KToolBarPopupMenus to be non-delayed which essentially removes the small arrow, how do you think that works?
I have also changed the display of URLs so that local URLs have the schema removed.

abetts added a subscriber: abetts.Feb 26 2019, 3:18 PM

UI-wise, I'm not thrilled by both buttons getting dropdown arrows to the right:

While it's undoubtedly more functional, it just looks... bad. :( And I'm not sure that the history menu feature is so important that it merits such an in-your-face UI. What most web browsers do these days is only show the history menu when you press-and-hold the back or forward button. What do you think about doing that instead? With the Breeze theme at least, when you give a ToolButton a menu, a tiny little dropdown arrow will appear in the corner of the button to alert the user that there's additional functionality there, which is much less visually obtrusive.

As for what to show in the menu, I would recommend removing the schema for local files, i.e. don't show file:// at the beginning.

Thanks for the quick feedback :)

I have changed the KToolBarPopupMenus to be non-delayed which essentially removes the small arrow, how do you think that works?
I have also changed the display of URLs so that local URLs have the schema removed.

What does it look like now?

UI-wise, I'm not thrilled by both buttons getting dropdown arrows to the right:

While it's undoubtedly more functional, it just looks... bad. :( And I'm not sure that the history menu feature is so important that it merits such an in-your-face UI. What most web browsers do these days is only show the history menu when you press-and-hold the back or forward button. What do you think about doing that instead? With the Breeze theme at least, when you give a ToolButton a menu, a tiny little dropdown arrow will appear in the corner of the button to alert the user that there's additional functionality there, which is much less visually obtrusive.

As for what to show in the menu, I would recommend removing the schema for local files, i.e. don't show file:// at the beginning.

Thanks for the quick feedback :)

I have changed the KToolBarPopupMenus to be non-delayed which essentially removes the small arrow, how do you think that works?
I have also changed the display of URLs so that local URLs have the schema removed.

What does it look like now?

So now the back/forward buttons doesn't change visual appearance, but they allow the user to get a popup menu with the history. What do you guys think?

Hmm, now the problem is that a normal click on the back and forward buttons opens the drop-down menu instead of just going back and forwards. What I think we want here is this:

  • Normal click on button: go back/forwards
  • Click-and-hold on button: show dropdown menu

Alternatively I could be okay with the original design that had visible dropdown arrows next to the buttons, if we can find a way to make them less visually obtrusive and more visibly connected to their parent buttons.

Hmm, now the problem is that a normal click on the back and forward buttons opens the drop-down menu instead of just going back and forwards. What I think we want here is this:

  • Normal click on button: go back/forwards
  • Click-and-hold on button: show dropdown menu

    Alternatively I could be okay with the original design that had visible dropdown arrows next to the buttons, if we can find a way to make them less visually obtrusive and more visibly connected to their parent buttons.

Maybe if we add long click and ALSO right click, you have two ways to get the history

Let me see if I can cook that up ;) I just played a bit with Chrome and that seems to work as you guys have suggested, being left click and hold will give the popup menu and right click instantly gives the popup menu, so I also agree that it is probably the most intuitive solution ;)

Currently I can't get KToolBarPopupAction to do this for me, either it shows the drop down arrow or it opens the menu when clicked, so I think I have to cook up my own solution.

Let me see if I can cook that up ;) I just played a bit with Chrome and that seems to work as you guys have suggested, being left click and hold will give the popup menu and right click instantly gives the popup menu, so I also agree that it is probably the most intuitive solution ;)

Firefox does this too FWIW.

Currently I can't get KToolBarPopupAction to do this for me, either it shows the drop down arrow or it opens the menu when clicked, so I think I have to cook up my own solution.

Might be worth even adding these new optional behaviors to the framework itself so other apps can use it too?

I have a prototype working of this, what I did is essentially disable drawing of the drop down arrow for QToolButton in the Breeze theme. Do you guys know if this is a path forward? Should it be done conditional, and if so on what?
This is the change i did in breezestyle.cpp

diff --git a/kstyle/breezestyle.cpp b/kstyle/breezestyle.cpp
index ed7d29b2..bcb7a788 100644
--- a/kstyle/breezestyle.cpp
+++ b/kstyle/breezestyle.cpp
@@ -6068,11 +6068,11 @@ namespace Breeze
         // copy option and alter palette
         QStyleOptionToolButton copy( *toolButtonOption );
 
-        const bool hasPopupMenu( toolButtonOption->features & QStyleOptionToolButton::MenuButtonPopup );
-        const bool hasInlineIndicator(
+        const bool hasPopupMenu(false/* toolButtonOption->features & QStyleOptionToolButton::MenuButtonPopup */);
+        const bool hasInlineIndicator(false/*
             toolButtonOption->features&QStyleOptionToolButton::HasMenu
             && toolButtonOption->features&QStyleOptionToolButton::PopupDelay
-            && !hasPopupMenu );
+            && !hasPopupMenu */);
 
         const auto buttonRect( subControlRect( CC_ToolButton, option, SC_ToolButton, widget ) );
         const auto menuRect( subControlRect( CC_ToolButton, option, SC_ToolButtonMenu, widget ) );

In terms of Breeze, I would like for toolbuttons with dropdown menus to always have a little drop-down arrow in the corner no matter whether the menu is activated on a regular click or a click-and-hold. See also https://bugs.kde.org/show_bug.cgi?id=344746

In terms of Breeze, I would like for toolbuttons with dropdown menus to always have a little drop-down arrow in the corner no matter whether the menu is activated on a regular click or a click-and-hold. See also https://bugs.kde.org/show_bug.cgi?id=344746

Now I am a little confused :) Didn't you just argue that these toolbuttons should _not_ have a little drop-down arrow? That was why I tried to remove them, hence the change in the Breeze theme (which is what actually adds them).

In terms of Breeze, I would like for toolbuttons with dropdown menus to always have a little drop-down arrow in the corner no matter whether the menu is activated on a regular click or a click-and-hold. See also https://bugs.kde.org/show_bug.cgi?id=344746

Now I am a little confused :) Didn't you just argue that these toolbuttons should _not_ have a little drop-down arrow? That was why I tried to remove them, hence the change in the Breeze theme (which is what actually adds them).

In general, I want toolbuttons that show menus when clicked to show arrows in the corner of their icon regardless of whether the menu appears on regular click or click-and-hold.

In this specific use case, I think omitting the arrow (if possible) is fine since that's what web browsers do. But if that's too technically thorny, I'm okay with following the desired convention and showing the arrow.

hallas added a comment.Mar 2 2019, 5:12 AM

In terms of Breeze, I would like for toolbuttons with dropdown menus to always have a little drop-down arrow in the corner no matter whether the menu is activated on a regular click or a click-and-hold. See also https://bugs.kde.org/show_bug.cgi?id=344746

Now I am a little confused :) Didn't you just argue that these toolbuttons should _not_ have a little drop-down arrow? That was why I tried to remove them, hence the change in the Breeze theme (which is what actually adds them).

In general, I want toolbuttons that show menus when clicked to show arrows in the corner of their icon regardless of whether the menu appears on regular click or click-and-hold.

In this specific use case, I think omitting the arrow (if possible) is fine since that's what web browsers do. But if that's too technically thorny, I'm okay with following the desired convention and showing the arrow.

That makes sense :D

I will look into adding a kind of Web Browser style for toolbuttons to have this functionality - Stay Tuned!

hallas added a comment.Mar 6 2019, 5:44 PM

I am now working on a prototype that consists of three pieces:

  1. A new class in KWidgetAddons called BrowserToolBarAction (naming suggestions are welcome :) ) - This will be a subclass of KToolBarPopupAction but it will not add any functionality, it is only used so that Breeze has a way to differentiate the way it is drawn.
  2. Special handling in BreezeStyle so that the BrowserToolBarAction is drawn without a drop down arrow, this is done by checking if the widget derives from BrowserToolBarAction
  3. Use the BrowserToolBarAction in Dolphin

Do you guys think this is a viable way? Is there a better way to get special handling in Breeze? I was also looking at using QObject::setProperty/QObject::property but it doesn't appear to be a common way of doing this. Any suggestions would be greatly appreciated :)

That seems viable.

However the more I think about it, the more I think maybe we should just stop trying to work around the default style and save ourselves this headache :). If this button has a menu that only appears on press-and-hold, maybe that's okay, and what we should do instead is try to make sure the little arrow in the corner of the icon isn't visually obtrusive and doesn't horizontally expand the clickable area.

hallas updated this revision to Diff 53905.Mar 14 2019, 6:14 PM

Changed KToolBarPopUpAction to have the stickyMenu attribute set as false.
Various cleanups.
Implement middle click handling.

@ngraham - I have found out that if I set the KToolBarPopupAction::delayed attribute to true and the KToolBarPopupAction::stickyMenu to false the breeze theme draws the arrows much more compact. I will just add a couple of screenshots so you can see the difference.

Before screenshot:

After screenshot:

Yeah. I'd prefer them even smaller, though. Ideally the button's width wouldn't increase at all; the arrow would just sit unobtrusively in the bottom-right corner. :)

I have made a small modification to the breeze theme, so now the arrow down indicator will not cause the tool button to be any wider, see this screenshot:

@ngraham - What do you think about this?

Definitely better. It looks like the icon gets pushed over to the left side though? I'd like it if the icon stayed where it is, and the arrow gets even smaller and even farther into the bottom-right corner. The arrow really doesn't need to be huge--even a little hint that there's a menu under there is enough IMO.

Hi there,
I just wanted to point out that Falkon, a KDE application, already implements this with a kind of buttons very similar (if not the same) to the ones Dolphin uses:


If it may be of any help, I believe the behaviour of these buttons is defined in https://cgit.kde.org/falkon.git/tree/src/lib/navigation/navigationbar.cpp. Falkon's drop-down history is compatible with both long click and right click.

If you ask me, adding a visual hint to let the user know there is a drop-down menu avaiable is unnecessary. In my honest opinion, any user who expects Dolphin to offer a navigation history will attempt right-clicking or holding one of these buttons. The only reason why they wouldn't try is because they already tried it in previous versions of Dolphin to no success, not because it's not intuitive. Thus I would find it a better idea to reach the userbase and inform them instead of having us to deal with unsightly or obtrusive dropdown arrows.

Also, thank you David Hallas for taking the lead in adding this feature.

Hi there,
I just wanted to point out that Falkon, a KDE application, already implements this with a kind of buttons very similar (if not the same) to the ones Dolphin uses:


If it may be of any help, I believe the behaviour of these buttons is defined in https://cgit.kde.org/falkon.git/tree/src/lib/navigation/navigationbar.cpp. Falkon's drop-down history is compatible with both long click and right click.

If you ask me, adding a visual hint to let the user know there is a drop-down menu avaiable is unnecessary. In my honest opinion, any user who expects Dolphin to offer a navigation history will attempt right-clicking or holding one of these buttons. The only reason why they wouldn't try is because they already tried it in previous versions of Dolphin to no success, not because it's not intuitive. Thus I would find it a better idea to reach the userbase and inform them instead of having us to deal with unsightly or obtrusive dropdown arrows.

Also, thank you David Hallas for taking the lead in adding this feature.

Hi @david.fontanals thanks for the pointers, I will take a look at the code for reference :)

@ngraham - what do you think? Should I go ahead and see if I can tweak the Breeze theme to paint the arrow indicator in the way you suggested, or should we go with the Falkon way? Do we have any other references/sources we can draw feedback from? From my testing I can see that Chrome doesn't draw any indication on the navigation buttons indicating there is a drop down menu available - the same goes for Firefox.

I don't hate the idea of not showing the arrows. We can do that.

However please do submit your patch that fixes https://bugs.kde.org/show_bug.cgi?id=344746! :)

I don't hate the idea of not showing the arrows. We can do that.

However please do submit your patch that fixes https://bugs.kde.org/show_bug.cgi?id=344746! :)

Hmm, I haven't done a patch that fixes https://bugs.kde.org/show_bug.cgi?id=344746 ;) I can see there is a looong discussion there, what is the bottom line of it? Should we maybe talk about it on an email thread? Or IRC?

The gist of it is that I want for Breeze-styled ToolButtons which display a menu when left-clicked normally (not pressed-and-held) to become wider and display a downward-pointing arrow, exactly like in your screenshot here:

But you're right, this is not relevant to this patch so we can discuss it elsewhere. I just figured that your patch was probably already 9/10ths of the way there. :)

Anyway, if you rejigger this patch to remove the downward-pointing arrows, I'll re-review and we can hopefully get it in for 19.04.0 if @elvisangelaccio agrees.

I couldn't help spending a little time looking more into this arrow stuff :D I have tried to adjust the down arrow so that it is much smaller and put in the lower right corner of the button, also it doesn't change the size of the button. I guess a screenshot says more then a 1000 words:

@ngraham - What do you think? Is this closer to what you have in mind? Also, should I post a new review where we can discuss this issue?

I couldn't help spending a little time looking more into this arrow stuff :D I have tried to adjust the down arrow so that it is much smaller and put in the lower right corner of the button, also it doesn't change the size of the button. I guess a screenshot says more then a 1000 words:

@ngraham - What do you think? Is this closer to what you have in mind? Also, should I post a new review where we can discuss this issue?

That's exactly what I had in mind, yes!!!! I might even make them a bit smaller and more in-the-cornery.

I couldn't help spending a little time looking more into this arrow stuff :D I have tried to adjust the down arrow so that it is much smaller and put in the lower right corner of the button, also it doesn't change the size of the button. I guess a screenshot says more then a 1000 words:

@ngraham - What do you think? Is this closer to what you have in mind? Also, should I post a new review where we can discuss this issue?

That's exactly what I had in mind, yes!!!! I might even make them a bit smaller and more in-the-cornery.

I have created a separate patch for the down-arrow-stuff here: D19890

So is there anything else that needs to be done for this change here?

ngraham accepted this revision as: VDG, ngraham.Mar 24 2019, 8:12 PM

Thanks, that's perfect. The UI for this is now flawless in my opinion. :) I'll hand this show back over to @elvisangelaccio for code review.

ognarb added a subscriber: ognarb.Mar 24 2019, 8:14 PM

About the UI: I'd also be in favor of not showing the arrows. It's not just Falkon that doesn't show them, but also Firefox and Chrome.

src/dolphinmainwindow.cpp
1682

Unrelated whitespace change

src/middleclickactioneventfilter.cpp
56–70

This could maybe go as the else branch of if (toolBar) ?

About the UI: I'd also be in favor of not showing the arrows. It's not just Falkon that doesn't show them, but also Firefox and Chrome.

After a lot of thought, I agree. I've reverted the Breeze change for the tiny arrows. Sorry for jerking you around here, @hallas! :( Would you mind updating the patch to the version that won't show any arrows at all?

About the UI: I'd also be in favor of not showing the arrows. It's not just Falkon that doesn't show them, but also Firefox and Chrome.

After a lot of thought, I agree. I've reverted the Breeze change for the tiny arrows. Sorry for jerking you around here, @hallas! :( Would you mind updating the patch to the version that won't show any arrows at all?

Hi @ngraham - no problem ;) Sometimes you have to jerk around a bit to find the best solution, especially when it comes to usability. I will update the patch.

About the UI: I'd also be in favor of not showing the arrows. It's not just Falkon that doesn't show them, but also Firefox and Chrome.

After a lot of thought, I agree. I've reverted the Breeze change for the tiny arrows. Sorry for jerking you around here, @hallas! :( Would you mind updating the patch to the version that won't show any arrows at all?

The version that doesn't show menu arrows doesn't have a delayed menu :/ There a few paths forward though:

  • Go with the version that doesn't draw any arrows but it is not a delayed menu meaning that the menu appears immediately when you click the back button. Personally I think this is a bad choice.
  • Modify/extend the Breeze theme to draw this button (via some special flag or something) without arrows, even though it has a delayed menu (maybe this should be a general change). Personally this would be my choice.
  • Implement our own button logic, similar to what is done in Falkon.

What do you guys ( @elvisangelaccio @ngraham ) think?

  • Modify/extend the Breeze theme to draw this button (via some special flag or something) without arrows, even though it has a delayed menu (maybe this should be a general change). Personally this would be my choice.

I agree, this is best. For inspiration, you might check out the following abandoned Breeze patch that did something similar and allowed opting out on a widget-by-widget basis: D13064 Maybe all we need to do is implement that opting-out mechanism for existing press-and-hold menu toolbuttons, and then set the hint in these buttons here.

This change is still WIP and is meant as a discussion point. I would like to get some feedback on this in general but most specifically around what to show in the navigation drop down? Currently I just put the full URL there but that is not very user friendly.

My thoughts on this (long sought :) feature:

  1. Since Firefox et al were already referenced, you guys surely noticed that pushing on either back or forward gives you the same dropdown-menu there. Can we expect this behavior also with this patch? (Didn't saw it mentioned yet).
  2. Re. list size: Firefox has 15 list items. So if you want to go back 20 entries, you have to invoke the menu twice (going -14 and then -6). Which I think is a reasonable approach. Showing all history entries at once, can get ugly quiet fast with a long history.
  3. Re. action title: I would be ok with just the (top) folder name. If you go with the path, I would make sure that there is a character limit, otherwise the menu could look quite ugly for users with long paths.

Below attached screenshot from firefox for reference:


Also notice the page icon in front of the title, which upon hovering changes to either the forward or backward arrow.

david.fontanals added a comment.EditedApr 14 2019, 3:44 PM

@richardl
As I see it, showing both forward and backward entries at the same time is a smart idea if your interface requires having a single button for this feature. Otherwise, it doesn't make much sense because one would never click on the backward navigation button to pick a forward entry. Having to see the forward entries first to decide which backward entry to pick is not a common case either (and vicecersa). Thus, in most cases you'd just have to deal with the drawback of limiting the amount of forward-only or backward-only entries that can be shown, which in addition is counter-productive if you want to roll back a lot of entries.

Falkon uses separate lists with a limit of 20 entries. It also has a "Clear history" button I have never used which forgets the backward/forward history for the current tab.

@richardl - thanks for the feedback! I tend to agree with @david.fontanals in that it makes most sense to only show the back history in the back button and the forward history in the forward button. This also seems to be the behavior of Chrome (though I don't know how much it matters :) ).

Regarding the title of each entry, I am pretty sure that this will be refined after this patch has landed and it gets some mileage :D But I think your point with having a limit is something we should look at. Otherwise I am curious to see how the feedback to this feature will be.

  • Modify/extend the Breeze theme to draw this button (via some special flag or something) without arrows, even though it has a delayed menu (maybe this should be a general change). Personally this would be my choice.

I agree, this is best. For inspiration, you might check out the following abandoned Breeze patch that did something similar and allowed opting out on a widget-by-widget basis: D13064 Maybe all we need to do is implement that opting-out mechanism for existing press-and-hold menu toolbuttons, and then set the hint in these buttons here.

@ngraham - I have implemented a solution based on D13064 where I add a new widget property that Breeze honors to disable the rendering of the arrow, but I am not sure what is the best way to actually set this property. Currently I have done a patch for KWidgetAddons::KToolBarPopupAction to set it, but I don't know if it would be better if Dolphin had a subclass of KWidgetAddons::KToolBarPopupAction and set the property there?

Here is the change in Breeze: F6773033
And the change in KWidgetAddons: F6773036

To test it you also need to change the following in Dolphin:

diff --git a/src/dolphinmainwindow.cpp b/src/dolphinmainwindow.cpp
index 035a174c6..c2015cb22 100644
--- a/src/dolphinmainwindow.cpp
+++ b/src/dolphinmainwindow.cpp
@@ -1252,6 +1252,7 @@ void DolphinMainWindow::setupActions()
     }
     m_backAction->setDelayed(true);
     m_backAction->setStickyMenu(false);
+    m_backAction->setMenuIndicator(false);
     connect(m_backAction, &QAction::triggered, this, &DolphinMainWindow::goBack);
     connect(m_backAction->menu(), &QMenu::aboutToShow, this, &DolphinMainWindow::slotAboutToShowBackPopupMenu);
     connect(m_backAction->menu(), &QMenu::triggered, this, &DolphinMainWindow::slotGoBack);
@@ -1287,6 +1288,7 @@ void DolphinMainWindow::setupActions()
     }
     m_forwardAction->setDelayed(true);
     m_forwardAction->setStickyMenu(false);
+    m_forwardAction->setMenuIndicator(false);
     connect(m_forwardAction, &QAction::triggered, this, &DolphinMainWindow::goForward);
     connect(m_forwardAction->menu(), &QMenu::aboutToShow, this, &DolphinMainWindow::slotAboutToShowForwardPopupMenu);
     connect(m_forwardAction->menu(), &QMenu::triggered, this, &DolphinMainWindow::slotGoForward);

What do you think? Should I post these patches in the respective repositories? Or doesn't it make sense to add this functionality to KWidgetAddons?

The Breeze patch looks sane. I'm torn on whether or not adding the new style to KWidgetsAddons makes sense though. Couldn't Dolphin just set the new _kde_toolButton_noMenuArrow on just these buttons?

@richardl
As I see it, showing both forward and backward entries at the same time is a smart idea if your interface requires having a single button for this feature. Otherwise, it doesn't make much sense because one would never click on the backward navigation button to pick a forward entry. Having to see the forward entries first to decide which backward entry to pick is not a common case either (and vicecersa). Thus, in most cases you'd just have to deal with the drawback of limiting the amount of forward-only or backward-only entries that can be shown, which in addition is counter-productive if you want to roll back a lot of entries.

I can't follow the remark about the single button, since there are clearly two buttons in firefox? Anyways, the rationale in firefox is IMHO that the user wants to jump to one point in the history, but if that's in the forward or backward history might not be known (in advance). (For a single (or double) forward/back one wouldn't invoke the menu anyways.) If you take this uncertainty into account the unified menu makes much more sense.

The Breeze patch looks sane. I'm torn on whether or not adding the new style to KWidgetsAddons makes sense though. Couldn't Dolphin just set the new _kde_toolButton_noMenuArrow on just these buttons?

I don't think I can, because the buttons are not created directly by Dolphin, but instead they are created by QToolbar using the KToolBarPopupAction::createWidget function. So therefore I think we would either need to handle it in KWidgetAddons or maybe do a subclass of KToolBarPopupAction in Dolphin which adds this property, what do you guys think?

The Breeze patch looks sane. I'm torn on whether or not adding the new style to KWidgetsAddons makes sense though. Couldn't Dolphin just set the new _kde_toolButton_noMenuArrow on just these buttons?

I don't think I can, because the buttons are not created directly by Dolphin, but instead they are created by QToolbar using the KToolBarPopupAction::createWidget function. So therefore I think we would either need to handle it in KWidgetAddons or maybe do a subclass of KToolBarPopupAction in Dolphin which adds this property, what do you guys think?

Ah yeah, that makes sense. A subclass in Dolphin that just adds that property probably makes sense.

Why would we want to handle it in dolphin? The proposed patch (F6773036) looks good to me, other applications could benefit from it without having to reinvent the wheel.

Sure thing. Please listen to @elvisangelaccio rather than my deluded ravings! :)

Sure thing. Please listen to @elvisangelaccio rather than my deluded ravings! :)

Ok - I will do that then!

I have pushed the Breeze change here: D20867 - please take a look!

What happens if the user is using an old version of breeze that doesn't have this new property? Will they get the arrows?

What happens if the user is using an old version of breeze that doesn't have this new property? Will they get the arrows?

Yes - they would simply get the arrows :)

BTW - I haven't tested this functionality with other styles then Breeze, but would assume that it should work fine there as well.

mart added a subscriber: mart.May 6 2019, 9:19 AM

needs button->setArrowType(Qt::ArrowType::NoArrow);

and then to debug and find why breeze still adds it (and check also what happens with other styles)

So what needs to happen here now? I'll admit I'm a bit lost. Do we need a Breeze patch, or can we set the correct hints here?

So what needs to happen here now? I'll admit I'm a bit lost. Do we need a Breeze patch, or can we set the correct hints here?

We can't create the hints here. The solution @mart suggested doesn't work. I have created a Breeze patch D20867 but it is still pending approval, I think that needs to be addressed before this patch can move forward.

But thanks for picking this one up :D