Most of the time we want an OverlaySheet to draw a close button so its clients don't have to. But sometimes clients want to draw their own close buttons, for example to use the OverlaySheet as a dialog box with a Cancel button (which we do in Discover). In these circumstances, we don't need OverlaySheet to draw its own close button; this patch makes that possible.
Details
- Reviewers
mart - Group Reviewers
Kirigami - Commits
- R169:1f96a170ebff: Allow OverlaySheet clients to omit the built-in close button
Tested in Discover after applying the patch and adding showCloseButton: false to DiscoverWindow.qml's proceedDialog:
Diff Detail
- Repository
- R169 Kirigami
- Branch
- overlay-sheet-without-close-button (branched from master)
- Lint
No Linters Available - Unit
No Unit Test Coverage
"Close button + cancel" is inherently redundant; a historical relic of the fact that dialog boxes have traditionally been separate windows, and windows have titlebars with fixed button arrangements (that said, in macOS, dialog boxes now have no window decorations). Having two visible methods of doing the exact same thing is pointless and confusing, and IMHO there's no reason to go out of our way to replicate this error now that we're using a convergent UI toolkit that has inline sheets.
I'm ok with it, tough i would change a bit the logic, as described
src/controls/templates/OverlaySheet.qml | ||
---|---|---|
117 | property alias showCloseButton: closeIcon.visible | |
304 | with the alias then we again have only: in this case, the default behavior of showCloseButton will be !Settings.isMobile, and then the developer could always override the behavior, breaking the binding to fore it always true or always false (so the property will work even on mobile) |