D12102: Port Language KCM to Qt Quick

2018-04-20 Thread Andres Betts
abetts added a comment. Here are additional mockups for thought.F5815917: Screen Shot 2018-04-20 at 10.54.07 PM.png F5815920: AddLanguageAnimation.mov REPOSITORY R119 Plasma Desktop REVISION DETAIL

D12311: Align lock icon with bold message text; reduce overall size of dialog

2018-04-20 Thread Stefan Brüns
bruns added a comment. Gnome-disks does, and so do other programs. REPOSITORY R121 Policykit (Polkit) KDE Agent BRANCH align-lock-icon (branched from master) REVISION DETAIL https://phabricator.kde.org/D12311 To: sharvey, davidedmundson, ngraham, abetts, #frameworks Cc: stikonas,

D12311: Align lock icon with bold message text; reduce overall size of dialog

2018-04-20 Thread Andrius Štikonas
stikonas added a comment. In D12311#250660 , @bruns wrote: > No, thats completely off, as thats the action you are authorizing. > > > Authorization is required to format disk WDC WD10EZEX-08M2NA0 > > I want it to show what it is asking

D12311: Align lock icon with bold message text; reduce overall size of dialog

2018-04-20 Thread Stefan Brüns
bruns added a comment. Its not available otherwise ... REPOSITORY R121 Policykit (Polkit) KDE Agent BRANCH align-lock-icon (branched from master) REVISION DETAIL https://phabricator.kde.org/D12311 To: sharvey, davidedmundson, ngraham, abetts, #frameworks Cc: stikonas, bruns,

D12311: Align lock icon with bold message text; reduce overall size of dialog

2018-04-20 Thread Luigi Toscano
ltoscano added a comment. In D12311#250660 , @bruns wrote: > No, thats completely off, as thats the action you are authorizing. > > > Authorization is required to format disk WDC WD10EZEX-08M2NA0 > > I want it to show what it is asking

D12311: Align lock icon with bold message text; reduce overall size of dialog

2018-04-20 Thread Stefan Brüns
bruns added a comment. No, thats completely off, as thats the action you are authorizing. > Authorization is required to format disk WDC WD10EZEX-08M2NA0 I want it to show what it is asking permission for, and to be specific - does it want to format the USB stick I just inserted, or

D12311: Align lock icon with bold message text; reduce overall size of dialog

2018-04-20 Thread Scott Harvey
sharvey added a comment. Hmm, I was actually leaning the other way. Ditch the generic boilerplate and keep the app-specific text. I think it's helpful when the dialog tells you why it appeared and what app/function is requesting your password. REPOSITORY R121 Policykit (Polkit) KDE Agent

D12311: Align lock icon with bold message text; reduce overall size of dialog

2018-04-20 Thread Nathaniel Graham
ngraham added a comment. So right now, we have two strings of text: - The top bold string comes from the app and differs on a per-app basis - The bottom long boilerplate string is from us, and shown all the time I like @abetts' idea: we should remove the app-specific text and make

D12311: Align lock icon with bold message text; reduce overall size of dialog

2018-04-20 Thread Scott Harvey
sharvey added a subscriber: stikonas. sharvey added a comment. Semi-related bug from @stikonas, while I'm on dialog duty: https://bugs.kde.org/show_bug.cgi?id=393355 REPOSITORY R121 Policykit (Polkit) KDE Agent BRANCH align-lock-icon (branched from master) REVISION DETAIL

D12311: Align lock icon with bold message text; reduce overall size of dialog

2018-04-20 Thread Scott Harvey
sharvey added a comment. The small-text boilerplate is definitely on the chopping block. @ltoscano makes a fine case for keeping the Details section. @bruns - Thanks for helping me find the source of the incoming messages. I hadn't gotten around to searching for them yet, but you saved

D12311: Align lock icon with bold message text; reduce overall size of dialog

2018-04-20 Thread Stefan Brüns
bruns added a comment. Btw, an easy way to trigger the dialog is e.g. $> pkcheck -u -p $$ -a org.freedesktop.udisks2.eject-media-system $$ is the PID of the shell. REPOSITORY R121 Policykit (Polkit) KDE Agent BRANCH align-lock-icon (branched from master) REVISION DETAIL

D12311: Align lock icon with bold message text; reduce overall size of dialog

2018-04-20 Thread Stefan Brüns
bruns added a comment. The "Authentication is required to ..." header line is directly sourced from the action definition below `/usr/share/polkit-1/actions/*` "Administrator password" is insufficient/wrong, as you can also (dependent on system configuration) authorize the actions as

D12311: Align lock icon with bold message text; reduce overall size of dialog

2018-04-20 Thread Andres Betts
abetts added a comment. In D12311#250637 , @bruns wrote: > If you really want to save some space (and probably make it easier for users to understand the dialog without getting lost), I think the whole > > > An application is attempting to

D12311: Align lock icon with bold message text; reduce overall size of dialog

2018-04-20 Thread Stefan Brüns
bruns added a comment. If you really want to save some space (and probably make it easier for users to understand the dialog without getting lost), I think the whole > An application is attempting to perform an action that requires privileges. > Authentication is required to perform

D9875: Extend parsing ssh prompt

2018-04-20 Thread Pali Rohár
pali updated this revision to Diff 32670. REPOSITORY R105 KDE SSH Password Dialog CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D9875?vs=32651=32670 REVISION DETAIL https://phabricator.kde.org/D9875 AFFECTED FILES src/main.cpp To: pali, fvogt Cc: fvogt, plasma-devel, ragreen,

D12311: Align lock icon with bold message text; reduce overall size of dialog

2018-04-20 Thread Luigi Toscano
ltoscano added a comment. Do you really want to remove the proper source of information in security dialog that asks you some additional credentials? Please keep it there. REPOSITORY R121 Policykit (Polkit) KDE Agent BRANCH align-lock-icon (branched from master) REVISION DETAIL

D12137: Use consistent spacing and units for suffixes in spinboxes

2018-04-20 Thread Friedrich W . H . Kossebau
This revision was automatically updated to reflect the committed changes. Closed by commit R114:442cf697be51: Use consistent spacing and units for suffixes in spinboxes (authored by kossebau). REPOSITORY R114 Plasma Addons CHANGES SINCE LAST UPDATE

D12171: Kicker: Make menus grow (to a limit) if the text doesn't fit on the default width

2018-04-20 Thread Eike Hein
hein accepted this revision. hein added a comment. This revision is now accepted and ready to land. Alrighty. REPOSITORY R119 Plasma Desktop BRANCH master REVISION DETAIL https://phabricator.kde.org/D12171 To: aacid, #plasma, hein Cc: fabianr, ngraham, davidedmundson, abetts,

D12311: Align lock icon with bold message text; reduce overall size of dialog

2018-04-20 Thread Nathaniel Graham
ngraham added a comment. In general it's okay to display nerdy technical information hidden away like this--as long as it's actually useful information! That's the real question. If it's of no real value to anyone for any use case that we can imaging, we can probably safely remove it.

D12311: Align lock icon with bold message text; reduce overall size of dialog

2018-04-20 Thread Scott Harvey
sharvey added a comment. Reviewers: as part of my task to redesign and tidy up this dialog box, I'm considering removing the Details button in the bottom left corner, along with the small pop-open panel that shows additional information. My argument is that the info in the Details panel is

D9875: Extend parsing ssh prompt

2018-04-20 Thread Fabian Vogt
fvogt added inline comments. INLINE COMMENTS > main.cpp:330 > +// Item could not be retrieved from wallet. Open dialog > +if (item.isEmpty()) { > +if (type == TypeConfirm) { Can you replace this if with if (!item.isEmpty()) { QTextStream(stdout) << item; return 0;

D12137: Use consistent spacing and units for suffixes in spinboxes

2018-04-20 Thread Albert Astals Cid
aacid added a comment. yeah i think that's easier to understand REPOSITORY R114 Plasma Addons BRANCH fixsecondssuffix REVISION DETAIL https://phabricator.kde.org/D12137 To: kossebau, #plasma, #localization, davidedmundson Cc: aacid, davidedmundson, plasma-devel, ragreen, Pitel,

D12102: Port Language KCM to Qt Quick

2018-04-20 Thread Eike Hein
hein added a comment. Currently we use the following inline messages to communicate stuff btw: - An info box shown when there's no languages available to add (i.e. not installed) - An info box shown no languages have been added (but some are available) - An error box when some added

D12102: Port Language KCM to Qt Quick

2018-04-20 Thread Andres Betts
abetts added a comment. I can make more REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D12102 To: hein, #kirigami, mart Cc: rkflx, aspotashev, davidedmundson, safaalfulaij, abetts, ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed,

D12102: Port Language KCM to Qt Quick

2018-04-20 Thread Eike Hein
hein added a comment. I think @rkflx gets that, he's talking just about the first list. I agree with his comments. Some more design mockups to work from would be nice :). REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D12102 To: hein, #kirigami, mart Cc:

D12102: Port Language KCM to Qt Quick

2018-04-20 Thread Andres Betts
abetts added a comment. In D12102#250486 , @rkflx wrote: > Hm, I could imagine regular users will have a hard time selecting their preferred language. They'll see a list of languages, and look for some kind of checkbox to "select what they

D12102: Port Language KCM to Qt Quick

2018-04-20 Thread Henrik Fehlauer
rkflx added a comment. Hm, I could imagine regular users will have a hard time selecting their preferred language. They'll see a list of languages, and look for some kind of checkbox to "select what they want". How should they realize that the top-most language will be used as the default,

D12102: Port Language KCM to Qt Quick

2018-04-20 Thread Eike Hein
hein added a comment. Updated screenshot, this time using the "Breeze" color scheme which has some contrast between window bg and view bg colors: F5815250: Screenshot_20180421_010244.png REPOSITORY R119 Plasma Desktop REVISION DETAIL

D12102: Port Language KCM to Qt Quick

2018-04-20 Thread Eike Hein
hein updated this revision to Diff 32652. hein added a comment. Set color set, add padding, wrap in ScrollView. This is all a bit hacky and mostly to get VDG feedback. Known problems: - When the lang list grows out of bounds you get double-scrollbars because I couldn't find a

D9875: Extend parsing ssh prompt

2018-04-20 Thread Pali Rohár
pali updated this revision to Diff 32651. pali added a comment. I changed those two booleans into enum. REPOSITORY R105 KDE SSH Password Dialog CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D9875?vs=32615=32651 REVISION DETAIL https://phabricator.kde.org/D9875 AFFECTED FILES

D12102: Port Language KCM to Qt Quick

2018-04-20 Thread Eike Hein
hein added a comment. OK, the plot thickens. Turns out this is a color scheme problem. We have "Default" and "Breeze" color schemes. They used to be (and are supposed to be) identical. But right now, in my Colors KCM, switching between Default and Breeze produces different results.

D12102: Port Language KCM to Qt Quick

2018-04-20 Thread Eike Hein
hein added a comment. Note btw that it's similarly broken for me in the new grid-based KCMs. We had the same discussion there where we want the grids to have the view background color behind them (white), but the grid in the Loon and Feel KCM for example is #fcfcfc for me. REPOSITORY

D12102: Port Language KCM to Qt Quick

2018-04-20 Thread Eike Hein
hein added a comment. There's no white bg in the latest screenshot, the entire thing is #fcfcfc (window bg color). REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D12102 To: hein, #kirigami, mart Cc: aspotashev, davidedmundson, safaalfulaij, abetts, ngraham,

D12102: Port Language KCM to Qt Quick

2018-04-20 Thread Nathaniel Graham
ngraham added a comment. Perhaps I'm blind, but in the latest screenshot it looks like there's already a white background. If anything, what we need is for the general background to be light gray like all the other KCMs, and for the list itself to retain its current white background

D12102: Port Language KCM to Qt Quick

2018-04-20 Thread Eike Hein
hein added a comment. I'm actually having no luck with the white field thing currently. I tried setting `Kirigami.Theme.colorSet: Kirigami.Theme.View` on the ListView and then setting `backgroundColor: Kirigami.Theme.viewBackgroundColor` on the SwipeListItem delegate, which I would expect

D12102: Port Language KCM to Qt Quick

2018-04-20 Thread Eike Hein
hein added a comment. I think we can do a white field. About the padding, I think it'd be cool if FormLayout could do this somehow. So it's automatic. (This code doesn't use FormLayout currently, though.) @mart? REPOSITORY R119 Plasma Desktop REVISION DETAIL

D12376: [ContextMenu Containment Action] Fix checking for KIOSK

2018-04-20 Thread Harald Sitter
sitter added a comment. The logout change seems fine. WRT run_command I wonder if it wouldn't be better to keep this backwards compatible `authorize(run_command) && authorizeAction(run_command)`. If a user previously restricted **only** `actions/run_command` that would, with the

D12375: [Power Management Engine] Fix kiosk restriction for lockscreen

2018-04-20 Thread Harald Sitter
sitter accepted this revision. sitter added a comment. This revision is now accepted and ready to land. LGTM (with the limited knowledge I just acquired in the couple of minutes I dug through kauth ;)) REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D12375

D12102: Port Language KCM to Qt Quick

2018-04-20 Thread Andres Betts
abetts added a comment. Could the list be enclosed by a white field? REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D12102 To: hein, #kirigami, mart Cc: aspotashev, davidedmundson, safaalfulaij, abetts, ngraham, plasma-devel, ragreen, Pitel, ZrenBot,

D12102: Port Language KCM to Qt Quick

2018-04-20 Thread Nathaniel Graham
ngraham added a comment. And if this is a Kirigami convergence thing, IMHO we should think about having Kirigami automatically add a frame and some side padding when on desktop platforms, where having a list span the entire horizontal width rarely looks good. REPOSITORY R119 Plasma

D12102: Port Language KCM to Qt Quick

2018-04-20 Thread Nathaniel Graham
ngraham added a comment. I'm not a huge fan of the fact that the list items take up the entire horizontal width and have no side borders. Makes things feel rather visually muddy, like things are too big and flowing into one another. Could we put that whole list inside a frame with a

D12102: Port Language KCM to Qt Quick

2018-04-20 Thread Eike Hein
hein added a comment. In D12102#250237 , @aspotashev wrote: > Looking at the screenshots, it's unclear which language do the up/down buttons belong to. Can we use drag'n'drop? Can we embed the buttons into the language row next to the language

D12102: Port Language KCM to Qt Quick

2018-04-20 Thread Eike Hein
hein updated this revision to Diff 32647. hein added a comment. Add context to strings. REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D12102?vs=32597=32647 BRANCH master REVISION DETAIL https://phabricator.kde.org/D12102 AFFECTED FILES

D12373: Klipper: Remove first history item on clipboard clear

2018-04-20 Thread Robert Hoffmann
hoffmannrobert added a comment. You are right, the "prevent empty clipboard" option needs to be set to false to make the "RemoveTopHistoryItemOnClear" option work correctly. REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D12373 To: hoffmannrobert Cc:

D12373: Klipper: Remove first history item on clipboard clear

2018-04-20 Thread David Edmundson
davidedmundson added a comment. We have two settings: The first, when the X selection owner gets cleared, replaces the clipboard with the top entry This one when the X selection owner gets cleared, deletes the top entry in the UI. Both default to on, which together makes no sense.

D12373: Klipper: Remove first history item on clipboard clear

2018-04-20 Thread Kai Uwe Broulik
broulik added a comment. Ideally, there was a mime data flag an application could set to indicate it wouldn't want it to end up in clipboard history but this is probably impossible to get applications to use.. REPOSITORY R120 Plasma Workspace REVISION DETAIL

D12376: [ContextMenu Containment Action] Fix checking for KIOSK

2018-04-20 Thread Kai Uwe Broulik
broulik created this revision. broulik added reviewers: Plasma, sitter. Restricted Application added a project: Plasma. Restricted Application added a subscriber: plasma-devel. broulik requested review of this revision. REVISION SUMMARY Anyone else checks for `run_command`. Docs explicitly

D12375: [Power Management Engine] Fix kiosk restriction for lockscreen

2018-04-20 Thread Kai Uwe Broulik
broulik created this revision. broulik added reviewers: Plasma, sitter. Restricted Application added a project: Plasma. Restricted Application added a subscriber: plasma-devel. broulik requested review of this revision. REVISION SUMMARY It is `action/lock_screen` according to documentation.

D12373: Klipper: Remove first history item on clipboard clear

2018-04-20 Thread Robert Hoffmann
hoffmannrobert added a comment. In D12373#250306 , @davidedmundson wrote: > Right, but pressing control+v again won't paste the item after clearing which I assumed was the intended goal Yes, pressing Control+V won't paste the item after

D12373: Klipper: Remove first history item on clipboard clear

2018-04-20 Thread David Edmundson
davidedmundson added a comment. Right, but pressing control+v again won't paste the item after clearing which I assumed was the intended goal REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D12373 To: hoffmannrobert Cc: davidedmundson, plasma-devel,

D12373: Klipper: Remove first history item on clipboard clear

2018-04-20 Thread Robert Hoffmann
hoffmannrobert added a comment. In D12373#250303 , @davidedmundson wrote: > Aren't you just duplicating the existing "prevent empty clipboard" option ? No, the "prevent empty clipboard" option ensures that the clipboard doesn't stay

D12373: Klipper: Remove first history item on clipboard clear

2018-04-20 Thread David Edmundson
davidedmundson added a comment. Aren't you just duplicating the existing "prevent empty clipboard" option ? REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D12373 To: hoffmannrobert Cc: davidedmundson, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai,

D12373: Klipper: Remove first history item on clipboard clear

2018-04-20 Thread Robert Hoffmann
hoffmannrobert created this revision. Restricted Application added a project: Plasma. Restricted Application added a subscriber: plasma-devel. hoffmannrobert requested review of this revision. REVISION SUMMARY Password manager tools like Keepassx offer an option to clear the

Re: Plasma Sprint topics

2018-04-20 Thread David Edmundson
​Reminder, if you have topics add them now. I've read everything there and made a very broad rough agenda that I've stuck at the top of the notes page; it should be vague enough to encompass everything whilst giving some structure. David

D12359: Also read sddm.conf.d config directories

2018-04-20 Thread Fabian Vogt
This revision was automatically updated to reflect the committed changes. Closed by commit R123:c879e4fa438f: Also read sddm.conf.d config directories (authored by fvogt). REPOSITORY R123 SDDM Configuration Panel (KCM) CHANGES SINCE LAST UPDATE

D12359: Also read sddm.conf.d config directories

2018-04-20 Thread Fabian Vogt
fvogt updated this revision to Diff 32628. fvogt added a comment. Improve the commit message. REPOSITORY R123 SDDM Configuration Panel (KCM) CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D12359?vs=32582=32628 BRANCH Plasma/5.12 REVISION DETAIL

D9875: Extend parsing ssh prompt

2018-04-20 Thread Fabian Vogt
fvogt added a comment. I gave it a quick test - works fine with ssh(-add). I didn't test anything else though. Code-wise it looks ok (as ok as the old code...), just a remark about the reference parameters. INLINE COMMENTS > main.cpp:44 > // has no i18n, so this should work for all

D12102: Port Language KCM to Qt Quick

2018-04-20 Thread Alexander Potashev
aspotashev added inline comments. INLINE COMMENTS > main.qml:59 > + > +header: Kirigami.Heading { text: i18n("Add Languages") } > + Please add context: i18nc("@title:window", "Add Languages") > main.qml:108 > + > +text: i18n("Add") > + Please add context for

D12102: Port Language KCM to Qt Quick

2018-04-20 Thread Alexander Potashev
aspotashev added a comment. Looking at the screenshots, it's unclear which language do the up/down buttons belong to. Can we use drag'n'drop? Can we embed the buttons into the language row next to the language name? REPOSITORY R119 Plasma Desktop REVISION DETAIL

D9875: Extend parsing ssh prompt

2018-04-20 Thread Fabian Vogt
fvogt added a comment. Use of `QStringLiteral` and `nullptr` in some cases, but maybe that's just phabricator doing git diff wrong. I'll try it out. REPOSITORY R105 KDE SSH Password Dialog REVISION DETAIL https://phabricator.kde.org/D9875 To: pali, fvogt Cc: fvogt, plasma-devel,

D9875: Extend parsing ssh prompt

2018-04-20 Thread Pali Rohár
pali added a comment. In D9875#250214 , @fvogt wrote: > too many unrelated changes in the diff Which are unrelated? I think all of them are needed. parsePrompt() was needed to be rewritten for support plain text inputs and yes/no

D9875: Extend parsing ssh prompt

2018-04-20 Thread Fabian Vogt
fvogt requested changes to this revision. fvogt added a comment. This revision now requires changes to proceed. Rebase looks ok - but now there are too many unrelated changes in the diff. Can you split them? REPOSITORY R105 KDE SSH Password Dialog REVISION DETAIL