D19502: Add a SearchField and PasswordField component
This revision was automatically updated to reflect the committed changes. Closed by commit R169:546f82f9e645: Add a SearchField and PasswordField component (authored by ognarb). REPOSITORY R169 Kirigami CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D19502?vs=54119=54353 REVISION DETAIL https://phabricator.kde.org/D19502 AFFECTED FILES examples/simpleexamples/MultipleColumnsGallery.qml kirigami.qrc kirigami.qrc.in src/controls/ActionTextField.qml src/controls/PasswordField.qml src/controls/SearchField.qml src/kirigamiplugin.cpp src/qmldir To: ognarb, #kirigami, ngraham, mart Cc: broulik, apol, plasma-devel, domson, dkardarakos, davidedmundson, mart, hein
D19502: Add a SearchField and PasswordField component
ognarb updated this revision to Diff 54119. ognarb added a comment. Rebase REPOSITORY R169 Kirigami CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D19502?vs=54117=54119 BRANCH dev REVISION DETAIL https://phabricator.kde.org/D19502 AFFECTED FILES examples/simpleexamples/MultipleColumnsGallery.qml kirigami.qrc kirigami.qrc.in src/controls/ActionTextField.qml src/controls/PasswordField.qml src/controls/SearchField.qml src/kirigamiplugin.cpp src/qmldir To: ognarb, #kirigami, ngraham, mart Cc: broulik, apol, plasma-devel, domson, dkardarakos, davidedmundson, mart, hein
D19502: Add a SearchField and PasswordField component
ognarb updated this revision to Diff 54117. ognarb added a comment. - Use qsTr instead of i18n - Kirigami version 2.7 -> 2.8 REPOSITORY R169 Kirigami CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D19502?vs=53106=54117 BRANCH dev REVISION DETAIL https://phabricator.kde.org/D19502 AFFECTED FILES examples/simpleexamples/MultipleColumnsGallery.qml kirigami.qrc kirigami.qrc.in src/controls/ActionTextField.qml src/controls/PasswordField.qml src/controls/SearchField.qml src/kirigamiplugin.cpp src/qmldir To: ognarb, #kirigami, ngraham, mart Cc: broulik, apol, plasma-devel, domson, dkardarakos, davidedmundson, mart, hein
D19502: Add a SearchField and PasswordField component
mart requested changes to this revision. mart added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > ognarb wrote in SearchField.qml:44 > Are you sure? It would mean that Kirigami should then depends on ki18n. And > them Kirigami wouldn't be a Tier 1 KDE framework anymore. > > @mart should I move this in the example code or it is fine with a translation > domain? no, it can't depend from ki18n. here we can onlt use qsTr REPOSITORY R169 Kirigami REVISION DETAIL https://phabricator.kde.org/D19502 To: ognarb, #kirigami, ngraham, mart Cc: broulik, apol, plasma-devel, domson, dkardarakos, davidedmundson, mart, hein
D19502: Add a SearchField and PasswordField component
ognarb added inline comments.
INLINE COMMENTS
> broulik wrote in SearchField.qml:44
> I think this is fine but may need a translation domain
> `i18nd("whatever_domain_kirigami_uses", "Search...")`
Are you sure? It would mean that Kirigami should then depends on ki18n. And
them Kirigami wouldn't be a Tier 1 KDE framework anymore.
@mart should I move this in the example code or it is fine with a translation
domain?
REPOSITORY
R169 Kirigami
REVISION DETAIL
https://phabricator.kde.org/D19502
To: ognarb, #kirigami, ngraham, mart
Cc: broulik, apol, plasma-devel, domson, dkardarakos, davidedmundson, mart, hein
D19502: Add a SearchField and PasswordField component
ognarb updated this revision to Diff 53106. ognarb marked 8 inline comments as done. ognarb edited the summary of this revision. ognarb added a comment. - Fix typos in doc - Remove edi-clear button in password field REPOSITORY R169 Kirigami CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D19502?vs=53093=53106 BRANCH dev REVISION DETAIL https://phabricator.kde.org/D19502 AFFECTED FILES examples/simpleexamples/MultipleColumnsGallery.qml kirigami.qrc kirigami.qrc.in src/controls/ActionTextField.qml src/controls/PasswordField.qml src/controls/SearchField.qml src/kirigamiplugin.cpp src/qmldir To: ognarb, #kirigami, ngraham, mart Cc: broulik, apol, plasma-devel, domson, dkardarakos, davidedmundson, mart, hein
D19502: Add a SearchField and PasswordField component
broulik added inline comments.
INLINE COMMENTS
> PasswordField.qml:27
> + *
> + * Example usage for the search field component:
> + * @code
password field
> PasswordField.qml:33
> + * id: passwordField
> + * onAccepted: console.log("Password text is " + passwordField.text)
> + * }
Better not print a password in an example code :)
> PasswordField.qml:51
> +},
> +Kirigami.Action {
> +iconName: "edit-clear"
We don't use clear buttons on password fields
> SearchField.qml:44
> +
> +placeholderText: i18n("Search...")
> +focusSequence: "Ctrl+F"
I think this is fine but may need a translation domain
`i18nd("whatever_domain_kirigami_uses", "Search...")`
> SearchField.qml:48
> +Kirigami.Action {
> +iconName: "edit-clear"
> +visible: root.text != ""
This doesn't mirror with right-to-left languages, run the app with `-reverse`
argument to try
> SearchField.qml:49
> +iconName: "edit-clear"
> +visible: root.text != ""
> +onTriggered: {
Prefer connecting to `length > 0` if available, avoids a string conversion and
passing around between C++ and JavaScript
REPOSITORY
R169 Kirigami
REVISION DETAIL
https://phabricator.kde.org/D19502
To: ognarb, #kirigami, ngraham, mart
Cc: broulik, apol, plasma-devel, domson, dkardarakos, davidedmundson, mart, hein
D19502: Add a SearchField and PasswordField component
ngraham added a reviewer: mart. ngraham added a comment. Wow, that was fast. I'm impressed. INLINE COMMENTS > PasswordField.qml:25 > +/** > + * This is a standart password text field. > + * `Standart` -> `Standard` > SearchField.qml:25 > +/** > + * This is a standart textfield following KDE HIG. Using Ctrl+F as focus > + * sequence and "Search..." as placeholder text. `Standart` -> `Standard` REPOSITORY R169 Kirigami REVISION DETAIL https://phabricator.kde.org/D19502 To: ognarb, #kirigami, ngraham, mart Cc: apol, plasma-devel, domson, dkardarakos, davidedmundson, mart, hein
D19502: Add a SearchField and PasswordField component
apol added inline comments.
INLINE COMMENTS
> PasswordField.qml:51
> +},
> +Kirigami.Action {
> +iconName: "edit-clear"
Looks off that the clear buttons are not aligned. Invert?
REPOSITORY
R169 Kirigami
REVISION DETAIL
https://phabricator.kde.org/D19502
To: ognarb, #kirigami, ngraham
Cc: apol, plasma-devel, domson, dkardarakos, davidedmundson, mart, hein
D19502: Add a SearchField and PasswordField component
ognarb created this revision. Herald added a project: Kirigami. Herald added a subscriber: plasma-devel. ognarb requested review of this revision. REVISION SUMMARY Provide a small abstraction over ActionTextField to implement consistent standard special text field **Warning:** I include a placeholder text in this two example, not sure if this is correct, because kirigami probably shouldn't include translatable content. TEST PLAN Compile, example work REPOSITORY R169 Kirigami BRANCH dev REVISION DETAIL https://phabricator.kde.org/D19502 AFFECTED FILES examples/simpleexamples/MultipleColumnsGallery.qml kirigami.qrc kirigami.qrc.in src/controls/ActionTextField.qml src/controls/PasswordField.qml src/controls/SearchField.qml src/kirigamiplugin.cpp src/qmldir To: ognarb Cc: plasma-devel, domson, dkardarakos, apol, davidedmundson, mart, hein
