Review: Approve Tested on my BQ, and looks good. Nice job! :D
Having a look at Niklas' review, I guess the "needs-fixing" is about the conflict with .pot, so there shouldn't be any problem in getting this branch merged. Anyway, here's some note for a further task: 1) I believe it's better to use the Qt APIs (instead of Ubuntu.Components.Clipboard) since currently the QML GUI does not handle the clipboard (everything is done internally in the terminal widget). This could change in future, but I don't think it's a good idea to change heavily the C++ terminal code yet (Filippo, the former terminal-app maintainer, aimed to keep the code in sync with the upstream project from LxQt). 2) I'd prefer to set "enabled: !terminal.isClipboardEmpty()" instead of "visible" in order to stay consistent with the rest of the platform (i.e. see TextField/TextArea popover). The entry in the popover should be always visible, but not triggerable if not necessary (I should check the UI specs though). 3) I'd expect to see a similar MP for the copy action too. Currently the action is still visible when no text is actually selected. (TBD in a different branch though) 4) Probably we'd like to redesign the whole popover, so that it looks similar to the popover used in the TextField/TextArea. We'd need to check if the component is publicly available or if we can get its style through a StyledItem. About the .pot conflict: If you don't need to update the translations in your branch, remember to "bzr revert po/*.pot" before committing. That way you're sure you won't get any annoying conflict. Thank you again! :) -- https://code.launchpad.net/~mcintire-evan/ubuntu-terminal-app/disable-paste/+merge/283244 Your team Ubuntu Terminal Developers is subscribed to branch lp:ubuntu-terminal-app. -- Mailing list: https://launchpad.net/~ubuntu-touch-coreapps-reviewers Post to : [email protected] Unsubscribe : https://launchpad.net/~ubuntu-touch-coreapps-reviewers More help : https://help.launchpad.net/ListHelp
