Differential D27840 Diff 80654 src/qmlcontrols/kcmcontrols/qml/SettingStateIndicator.qml

Changeset View

Standalone View

src/qmlcontrols/kcmcontrols/qml/SettingStateIndicator.qml

This file was added.

1		/*
2		Copyright (c) 2020 Kevin Ottens <kevin.ottens@enioka.com>
3
4		This library is free software; you can redistribute it and/or
5		modify it under the terms of the GNU Library General Public
6		License version 2 as published by the Free Software Foundation.
7
8		This library is distributed in the hope that it will be useful,
9		but WITHOUT ANY WARRANTY; without even the implied warranty of
10		MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
11		Library General Public License for more details.
12
13		You should have received a copy of the GNU Library General Public License
14		along with this library; see the file COPYING.LIB. If not, write to
15		the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
16		Boston, MA 02110-1301, USA.
17		*/
18
19		import QtQuick 2.8
20		import QtQuick.Controls 2.12
21		import org.kde.kcm 1.3 as KCM
22		import org.kde.kirigami 2.2 as Kirigami
23
24		/**
25		* A simple icon which automatically picks an icon, displays or hides itself
26		* depending on a setting state.
27		*
28		* @since 5.70
29		*/
30		ToolButton {
31		id: root
	davidedmundsonUnsubmitted Not Done This is a faux-button. Which means it needs all the relevant: Accessible.blah roles to be manually added Should it be in the tab focus chain? (or we could just inherit ToolButton) davidedmundson: This is a faux-button. Which means it needs all the relevant: Accessible.blah roles to be…
	ervinAuthorUnsubmitted Not Done Note this is the case for the QtWidgets implementation as well. I switched to ToolButton here but I likely need to do something similar in my other patch... would have been nice to have this comment earlier there (it's been ready a bit longer), you guys have a bias against widgets based patches. ;-) I think it shouldn't temper with focus at all though, this is almost impossible to get correct in that context I think, so it'll simply reject focus (not too much of an issue in that context IMHO). ervin: Note this is the case for the QtWidgets implementation as well. I switched to ToolButton here…
32
33		/**
34		* settingState: KCM.SettingStateProxy
35		* The state used by the indicator to decides if and how it is displayed
36		*/
37		property KCM.SettingStateProxy settingState
38
39		visible: settingState && !settingState.defaulted
	davidedmundsonUnsubmitted Done Setting root width/height inside a component is almost always not what you want. Use implicitWidth/implicitHeight. davidedmundson: Setting root width/height inside a component is almost always not what you want. Use…
	broulikUnsubmitted Done Probably should be `Kirigami.Units.iconSizes.small`. Generally, when creating reusable components, avoid giving them a `width` and `height`. Instead, define an `implicitWidth` and `implicitHeight`. broulik: Probably should be `Kirigami.Units.iconSizes.small`. Generally, when creating reusable…
40		focusPolicy: Qt.NoFocus
41
	broulikUnsubmitted Done Perhaps bind to `icon.valid`? broulik: Perhaps bind to `icon.valid`?
	ervinAuthorUnsubmitted Not Done Done completely differently with ToolButton anyway. ervin: Done completely differently with ToolButton anyway.
42		implicitWidth: icon.width + Kirigami.Units.largeSpacing
43		implicitHeight: icon.height + Kirigami.Units.largeSpacing
44
45		icon.name: "edit-reset"
46		icon.width: Kirigami.Units.iconSizes.small
47		icon.height: Kirigami.Units.iconSizes.small
48
49		onClicked: settingState.resetToDefault()
	broulikUnsubmitted Done You could make the `root` `Item` a `MouseArea` instead. Also, I think this should get some kind of hover and/or pressed indication? Also, what about `Accessible` and keyboard focus? Should this be a proper `ToolButton` control instead? broulik: You could make the `root` `Item` a `MouseArea` instead. Also, I think this should get some kind…
	ervinAuthorUnsubmitted Not Done See my reply to @davidedmundson above, I switched to ToolButton. ervin: See my reply to @davidedmundson above, I switched to ToolButton.
50		}