D10475: Make it possible to show the title despite having ctx actions

2018-03-05 Thread Aleix Pol Gonzalez
This revision was automatically updated to reflect the committed changes. Closed by commit R169:9665afb2a594: Make it possible to show the title despite having ctx actions (authored by apol). REPOSITORY R169 Kirigami CHANGES SINCE LAST UPDATE

D10475: Make it possible to show the title despite having ctx actions

2018-03-05 Thread Nathaniel Graham
ngraham accepted this revision. ngraham added a comment. This revision is now accepted and ready to land. Looks good now! With your latest update, the right padding issue now actually resolved. The follow-up patch only needs to fix the left padding for the menu items that don't have

D10475: Make it possible to show the title despite having ctx actions

2018-03-05 Thread Aleix Pol Gonzalez
apol updated this revision to Diff 28727. apol added a comment. Improve how we calculate the header title. Let's fix the right spacing issue in a separate review REPOSITORY R169 Kirigami CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10475?vs=28726=28727 BRANCH

D10475: Make it possible to show the title despite having ctx actions

2018-03-05 Thread Nathaniel Graham
ngraham added a comment. The problems persist with `QT_SCALE_FACTOR=1.5`, and additionally the titles get inappropriately elided: F5741717: Persists.png REPOSITORY R169 Kirigami REVISION DETAIL https://phabricator.kde.org/D10475 To: apol,

D10475: Make it possible to show the title despite having ctx actions

2018-03-05 Thread Aleix Pol Gonzalez
apol updated this revision to Diff 28726. apol added a comment. Odd, they both look rather small to me, probably a high dpi issue to look into... REPOSITORY R169 Kirigami CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10475?vs=28723=28726 BRANCH arcpatch-D10475_1 REVISION

D10475: Make it possible to show the title despite having ctx actions

2018-03-05 Thread Nathaniel Graham
ngraham added a comment. Hmm, now there's too much side padding! F5741687: Now there's too much.png Can we make the padding identical on the top, bottom, and right side? Also, as you can see in the above screenshot, the menu items without

D10475: Make it possible to show the title despite having ctx actions

2018-03-05 Thread Aleix Pol Gonzalez
apol updated this revision to Diff 28723. apol added a comment. Address Nate's comments REPOSITORY R169 Kirigami CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10475?vs=28393=28723 BRANCH arcpatch-D10475_1 REVISION DETAIL https://phabricator.kde.org/D10475 AFFECTED FILES

D10475: Make it possible to show the title despite having ctx actions

2018-03-02 Thread Nathaniel Graham
ngraham added a comment. Generally looks good. A few UI comments: - Let's use a hamburger icon as the default icon, rather than the three-dot icon. That's used elsewhere on the list to expose hidden actions; here, it's opening a standard drop-down menu, so let's use the more standard

D10475: Make it possible to show the title despite having ctx actions

2018-03-02 Thread Aleix Pol Gonzalez
apol updated this revision to Diff 28393. apol added a comment. Give the title some more space if there's no actions Someone please review the change REPOSITORY R169 Kirigami CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10475?vs=28334=28393 BRANCH arcpatch-D10475_1

D10475: Make it possible to show the title despite having ctx actions

2018-03-01 Thread Aleix Pol Gonzalez
apol updated this revision to Diff 28334. apol added a comment. fix logic REPOSITORY R169 Kirigami CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10475?vs=28333=28334 BRANCH arcpatch-D10475_1 REVISION DETAIL https://phabricator.kde.org/D10475 AFFECTED FILES

D10475: Make it possible to show the title despite having ctx actions

2018-03-01 Thread Aleix Pol Gonzalez
apol updated this revision to Diff 28333. apol added a comment. Tweaking REPOSITORY R169 Kirigami CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10475?vs=28332=28333 BRANCH arcpatch-D10475_1 REVISION DETAIL https://phabricator.kde.org/D10475 AFFECTED FILES

D10475: Make it possible to show the title despite having ctx actions

2018-03-01 Thread Aleix Pol Gonzalez
apol updated this revision to Diff 28332. apol added a comment. Rebase REPOSITORY R169 Kirigami CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10475?vs=28330=28332 BRANCH arcpatch-D10475_1 REVISION DETAIL https://phabricator.kde.org/D10475 AFFECTED FILES

D10475: Make it possible to show the title despite having ctx actions

2018-02-28 Thread Aleix Pol Gonzalez
apol updated this revision to Diff 28305. apol added a comment. Improve usability of the patch overall REPOSITORY R169 Kirigami CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10475?vs=27825=28305 BRANCH arcpatch-D10475_1 REVISION DETAIL https://phabricator.kde.org/D10475

D10475: Make it possible to show the title despite having ctx actions

2018-02-22 Thread Aleix Pol Gonzalez
apol updated this revision to Diff 27825. apol added a comment. Center the title and leave actions to the left and ctx actions to the right REPOSITORY R169 Kirigami CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10475?vs=27623=27825 BRANCH arcpatch-D10475 REVISION DETAIL

D10475: Make it possible to show the title despite having ctx actions

2018-02-22 Thread Aleix Pol Gonzalez
apol added a comment. In D10475#211781 , @abetts wrote: > I am in favor of placing the Titles left aligned, but only if we don't put more than 1 control to the left of the title. If we can't compromise on that, another option is to keep titles

D10475: Make it possible to show the title despite having ctx actions

2018-02-22 Thread Andres Betts
abetts added a comment. I am in favor of placing the Titles right aligned, but only if we don't put more than 1 control to the right of the title. If we can't compromise on that, another option is to keep titles center-aligned. REPOSITORY R169 Kirigami REVISION DETAIL

D10475: Make it possible to show the title despite having ctx actions

2018-02-22 Thread Nathaniel Graham
ngraham added a comment. The title is still the primary focus for this header UI, which means (for RTL languages at least), it needs to be left or center aligned. This means the actions have to be on the right, to make sure your eyes don't see them before the title. @abetts, what do

D10475: Make it possible to show the title despite having ctx actions

2018-02-20 Thread Aleix Pol Gonzalez
apol updated this revision to Diff 27623. apol added a comment. Rebase on master REPOSITORY R169 Kirigami CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10475?vs=27167=27623 BRANCH arcpatch-D10475 REVISION DETAIL https://phabricator.kde.org/D10475 AFFECTED FILES

D10475: Make it possible to show the title despite having ctx actions

2018-02-20 Thread Aleix Pol Gonzalez
apol added a comment. Now that we have a menu, we definitely need to look into this. Without patch: odd F5720319: Screenshot_20180220_154236.png With patch: still odd F5720335: Screenshot_20180220_155606.png

D10475: Make it possible to show the title despite having ctx actions

2018-02-15 Thread Marco Martin
mart added a comment. In D10475#206002 , @ngraham wrote: > F5710868: Too much on the left 1.png It looks so unbalanced. would probably be better < | VLC media player | icons REPOSITORY R169

D10475: Make it possible to show the title despite having ctx actions

2018-02-14 Thread Nathaniel Graham
ngraham added a comment. Don't worry Henrik, you're all good. The underlying issue here is that we haven't actually done the necessary design work for this feature yet. The appropriate Kirigami HIG section is empty: https://community.kde.org/KDE_Visual_Design_Group/KirigamiHIG#Bars If

D10475: Make it possible to show the title despite having ctx actions

2018-02-14 Thread Henrik Fehlauer
rkflx added a comment. Oops, I really didn't want to block progress here. I think the only thing which needs clarification is what "title" entails in relation to the rest of the app, perhaps with a couple of examplary use cases. Another approach would be to define the text element as a

D10475: Make it possible to show the title despite having ctx actions

2018-02-14 Thread Nathaniel Graham
ngraham added a comment. I agree with @rkflx. I think we need to step back here and do some more long, hard design work before we rush to implementation. The proposed approach will not work for most narrow headers as there will simply not be enough room for navigation buttons, a title,

D10475: Make it possible to show the title despite having ctx actions

2018-02-14 Thread Henrik Fehlauer
rkflx added a comment. @ngraham > In this patch, we are trying to solve the general case of how a navigation header with context actions should look. I see, but I'm afraid with only a single example (which should be solved differently anyway) it is going to be hard to reach a

D10475: Make it possible to show the title despite having ctx actions

2018-02-14 Thread Aleix Pol Gonzalez
apol updated this revision to Diff 27167. apol added a comment. Fix placement of the navigation arrows REPOSITORY R169 Kirigami CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10475?vs=27129=27167 BRANCH master REVISION DETAIL https://phabricator.kde.org/D10475 AFFECTED

D10475: Make it possible to show the title despite having ctx actions

2018-02-14 Thread Nathaniel Graham
ngraham added a comment. @rkflx Not naive at all! In this patch, we are trying to solve the general case of how a navigation header with context actions should look. Once we implement this control in Discover, we will //definitely// be removing the title from the app page's use of it,

D10475: Make it possible to show the title despite having ctx actions

2018-02-14 Thread Henrik Fehlauer
rkflx added a comment. Excuse the very naive question, but why is the title duplicated in the top bar in the first place? It looks very cluttered, unaligned, and jumps around depending on which buttons are in use. It seems the only situation a title on top would be useful is when the

D10475: Make it possible to show the title despite having ctx actions

2018-02-13 Thread Nathaniel Graham
ngraham added a reviewer: abetts. ngraham added a comment. Great, now we have the titles on the left, like @mart wants. We're getting there! There are a couple remaining issues: - Needs a re-base as it doesn't apply cleanly onto current master - Now everything in the header is

D10475: Make it possible to show the title despite having ctx actions

2018-02-13 Thread Aleix Pol Gonzalez
apol updated this revision to Diff 27129. apol added a comment. Make the title actually stay to the left REPOSITORY R169 Kirigami CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10475?vs=27083=27129 BRANCH master REVISION DETAIL https://phabricator.kde.org/D10475 AFFECTED

D10475: Make it possible to show the title despite having ctx actions

2018-02-13 Thread Nathaniel Graham
ngraham added a comment. For me this patch still has the titles aligned to the right, even after your change: F5710374: right.png But as currently implemented, if we move the title over to the left as I'm asking for, then the above image would

D10475: Make it possible to show the title despite having ctx actions

2018-02-13 Thread Nathaniel Graham
ngraham added a comment. No, but in Desktop mode, if the window is not widescreen, then the left side of the header currently gets navigation buttons: Back when you can go back, and after the first time you go back, Forward is shown, too. REPOSITORY R169 Kirigami REVISION DETAIL

D10475: Make it possible to show the title despite having ctx actions

2018-02-13 Thread Aleix Pol Gonzalez
apol added a comment. In D10475#205609 , @ngraham wrote: > Also, what happens to the navigation buttons if you're not in widescreen view? You get navigation buttons, and then context actions, and then the title? That'll be an awful lot of

D10475: Make it possible to show the title despite having ctx actions

2018-02-13 Thread Aleix Pol Gonzalez
apol updated this revision to Diff 27083. apol added a comment. Keep the title aligned to the left REPOSITORY R169 Kirigami CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10475?vs=27074=27083 BRANCH master REVISION DETAIL https://phabricator.kde.org/D10475 AFFECTED FILES

D10475: Make it possible to show the title despite having ctx actions

2018-02-13 Thread Nathaniel Graham
ngraham requested changes to this revision. ngraham added a comment. This revision now requires changes to proceed. This patch makes the title right-aligned, which is non-standard with anything. Let's keep it left-aligned for now. Also, what happens to the navigation buttons if you're not in

D10475: Make it possible to show the title despite having ctx actions

2018-02-13 Thread Marco Martin
mart accepted this revision. This revision is now accepted and ready to land. REPOSITORY R169 Kirigami BRANCH master REVISION DETAIL https://phabricator.kde.org/D10475 To: apol, #kirigami, mart Cc: mart, ngraham, plasma-devel, apol, davidedmundson, hein

D10475: Make it possible to show the title despite having ctx actions

2018-02-13 Thread Aleix Pol Gonzalez
apol updated this revision to Diff 27074. apol added a comment. Have the 3-dots button rightmost REPOSITORY R169 Kirigami CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10475?vs=27033=27074 BRANCH master REVISION DETAIL https://phabricator.kde.org/D10475 AFFECTED FILES

D10475: Make it possible to show the title despite having ctx actions

2018-02-13 Thread Nathaniel Graham
ngraham edited the summary of this revision. REPOSITORY R169 Kirigami REVISION DETAIL https://phabricator.kde.org/D10475 To: apol, #kirigami Cc: mart, ngraham, plasma-devel, apol, davidedmundson, hein

D10475: Make it possible to show the title despite having ctx actions

2018-02-13 Thread Marco Martin
mart added a comment. I like it! tough i think the more actions button should always be on the right of everything else, including the title REPOSITORY R169 Kirigami REVISION DETAIL https://phabricator.kde.org/D10475 To: apol, #kirigami Cc: mart, ngraham, plasma-devel, apol,

D10475: Make it possible to show the title despite having ctx actions

2018-02-12 Thread Aleix Pol Gonzalez
apol added a comment. Here you can see what Discover looks like once it's adopted. F5708931: kirigami-normal.png F5708930: kirigami-tonofactions.png REPOSITORY R169 Kirigami REVISION DETAIL

D10475: Make it possible to show the title despite having ctx actions

2018-02-12 Thread Aleix Pol Gonzalez
apol updated this revision to Diff 27033. apol added a comment. Remove unrelated changes REPOSITORY R169 Kirigami CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10475?vs=27032=27033 BRANCH master REVISION DETAIL https://phabricator.kde.org/D10475 AFFECTED FILES

D10475: Make it possible to show the title despite having ctx actions

2018-02-12 Thread Aleix Pol Gonzalez
apol created this revision. apol added a reviewer: Kirigami. Restricted Application added a project: Kirigami. Restricted Application added a subscriber: plasma-devel. apol requested review of this revision. REVISION SUMMARY At the moment, the ToolBarApplicationHeader doesn't show any title if