D27204: Fix the KNSQuick Dialog's close title and basic layout

[Dan Leinir Turthra Jensen]

This revision was automatically updated to reflect the committed changes.
Closed by commit R304:fd5b37818e6a: Fix the KNSQuick Dialogs close title 
and basic layout (authored by leinir).

REPOSITORY
  R304 KNewStuff

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27204?vs=75151=75165

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

AFFECTED FILES
  src/qtquick/qml/Dialog.qml

To: leinir, #frameworks, ngraham
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D27204: Fix the KNSQuick Dialog's close title and basic layout

[Dan Leinir Turthra Jensen]

leinir added a comment.


  In D27204#607325 , @ngraham wrote:
  
  > LGTM and fixes all the bugs! Thanks.
  
  
  
  
Thanks! :D
  
  > If it lands quickly, I wonder if it would be worth it to try and squeeze 
this into Frameworks 5.67 so all the people using this dialog in Plasma 5.18 
see the polished-up version from the start.
  
  
  
i was hoping the same - definitely would be worth it :)

REPOSITORY
  R304 KNewStuff

BRANCH
  fix-dialog-background-and-button (branched from master)

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

To: leinir, #frameworks, ngraham
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D27204: Fix the KNSQuick Dialog's close title and basic layout

[Nathaniel Graham]

ngraham accepted this revision.
ngraham added a comment.
This revision is now accepted and ready to land.


  LGTM and fixes all the bugs! Thanks.
  
  If it lands quickly, I wonder if it would be worth it to try and squeeze this 
into Frameworks 5.67 so all the people using this dialog in Plasma 5.18 see the 
polished-up version from the start.

REPOSITORY
  R304 KNewStuff

BRANCH
  fix-dialog-background-and-button (branched from master)

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

To: leinir, #frameworks, ngraham
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D27204: Fix the KNSQuick Dialog's close title and basic layout

[Dan Leinir Turthra Jensen]

leinir edited the test plan for this revision.
leinir added reviewers: Frameworks, ngraham.

REPOSITORY
  R304 KNewStuff

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

To: leinir, #frameworks, ngraham
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D27204: Fix the KNSQuick Dialog's close title and basic layout

[Dan Leinir Turthra Jensen]

leinir created this revision.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
leinir requested review of this revision.

REVISION SUMMARY
  This resolves several issues found in the KNSQuick Dialog component.
  
  Specifically it puts a background in the dialog (as a rectangle),
  since the QtQuick Dialogs base dialog lacks such a thing by default,
  and offers no background property to rectify it.
  
  It further changes the Close button from a custom one to using a
  default button provided by the various underlying systems. This
  is more fault proof than the previous incarnation, but requires
  that the escape button press is handled separately.
  
  Finally it sets a sensible default size (previously the dialog
  was made large by the layout, but as a side effect of the layouting
  now being done without a Layout (which we could, but which would make
  the code enormously convoluted for little gain), we can now set a
  set of implicit width and height on the content item, which results
  in both a sensible default size and a dialogue that will resize down
  the way one would expect it to).
  
  This could have been done as separate patches, but they're too
  intertwined for that to feel in any way reasonable to do...
  
  BUG:415606
  BUG:416354
  BUG:414682

REPOSITORY
  R304 KNewStuff

BRANCH
  fix-dialog-background-and-button (branched from master)

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

AFFECTED FILES
  src/qtquick/qml/Dialog.qml

To: leinir
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns