Fix the header layouts for EntryDetails and Page components
ClosedPublic

Authored by leinir on Oct 30 2019, 10:29 AM.

Details

Summary

This ensures that the header is the correct width, and further that
the text does not disappear when there is no space for the text (so
that it elides as intended).

BUG:413433
BUG:413440

Test Plan

Without patch: Resize a dialog to make the text not fit, and it disappears
With patch: Resize dialog, text is elided as one would expect, and the header row takes up the correct amount of space

Diff Detail

Repository
R304 KNewStuff
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
leinir created this revision.Oct 30 2019, 10:29 AM
Restricted Application added a project: Frameworks. · View Herald TranscriptOct 30 2019, 10:29 AM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
leinir requested review of this revision.Oct 30 2019, 10:29 AM
leinir edited the test plan for this revision. (Show Details)Oct 30 2019, 10:30 AM
leinir added reviewers: ngraham, KNewStuff.
ngraham requested changes to this revision.Oct 30 2019, 3:53 PM

Close, but not quite:

src/qtquick/qml/EntryDetails.qml
94

I don't think this works, since it's not inside another layout. Instead maybe anchors.fill: parent would work better?

src/qtquick/qml/Page.qml
98

ditto

This revision now requires changes to proceed.Oct 30 2019, 3:53 PM
ahiemstra added inline comments.
src/qtquick/qml/EntryDetails.qml
94

It does not. I am also unsure whether anchors.fill will work, since the titleDelegate is instantiated by a loader and its sizing behaviour is somewhat fuzzy. The only option that is guaranteed to work is setting a proper implicitWidth for the delegate. I actually spent some time trying to get balanced auto sizing behaviour working for titles when doing the ActionToolBar work, but in the end gave up because things get really tricky when you have two items that resize based on the size of the other item.

leinir updated this revision to Diff 69067.Oct 30 2019, 4:30 PM

Based on @ahiemstra's comments on the sizing logic, this seems to work...

  • Switch to an implicitWidth based approach
leinir added inline comments.Oct 30 2019, 4:32 PM
src/qtquick/qml/EntryDetails.qml
94

That is terribly annoying... So, with a bit of knowledge about this specific pair of pages, i've switched this to using implicitWidths set to the width of the page itself, so... let's see if that works a bit better. (which, incidentally, it does here, at least... but so did the previous approach...)

Now that's fixed on the main page but it's missing margins on the right side:

And it's still broken on details pages:

Now that's fixed on the main page but it's missing margins on the right side:

Right, progress... uncomfortably, it /has/ been fine here, but after pulling recent changes in from Kirigami i can now at least reproduce your issues... which is fun. After a touch of looking around, it turns out that D24634 is what broke this. So that's fun. @ahiemstra do you have any insights here, perhaps, which would save me hunting around /too/ aimlessly? :)

leinir updated this revision to Diff 69281.Nov 4 2019, 4:09 PM

Thanks to @ahiemstra for helping out with the Kirigami side :)

  • Switch to using layouts (and massively simplify the sizing logic)

With that Kirigami patch, this *almost* works. There's still an empty area on the right:

leinir added a comment.Nov 4 2019, 4:23 PM

With that Kirigami patch, this *almost* works. There's still an empty area on the right:

We're working on that this moment in the Kirigami chat :)

With that Kirigami patch, this *almost* works. There's still an empty area on the right:

The other patch updated and whatnot, and it now looks like so:

In other words, as it should! :) (and with much simplified code as well, which is always a bonus ;) )

ahiemstra added inline comments.Nov 5 2019, 11:18 AM
src/qtquick/qml/EntryDetails.qml
93–94

Hmm, with the new toolbar code, there's actually little reason to use a custom delegate here, since the toolbar header already uses a title + tool buttons style. You could convert the three toolbuttons to actions and drop the rest of the delegate.

leinir added inline comments.Nov 5 2019, 12:31 PM
src/qtquick/qml/EntryDetails.qml
93–94

Ooh... you know, that's a very good idea :)

It'd be even better if we could work out how to do that with the bits on the front page as well... this /is/ intended to work for convergence and whatnot - but let's poke at that after we've got this bit sorted and such :)

leinir updated this revision to Diff 69319.Nov 5 2019, 12:40 PM
leinir marked 6 inline comments as done.
  • Remove the custom title delegate on EntryDetails
ngraham accepted this revision.Nov 5 2019, 2:54 PM
This revision is now accepted and ready to land.Nov 5 2019, 2:54 PM
This revision was automatically updated to reflect the committed changes.