Merge lp:~robert-ancell/unity-control-center/screen-sharing into lp:unity-control-center
| Status: | Merged |
|---|---|
| Merge reported by: | Jeremy Bicha |
| Merged at revision: | not available |
| Proposed branch: | lp:~robert-ancell/unity-control-center/screen-sharing |
| Merge into: | lp:unity-control-center |
| Diff against target: |
1362 lines (+1271/-1) 13 files modified
configure.ac (+3/-0) panels/Makefile.am (+2/-1) panels/screen-sharing/Makefile.am (+39/-0) panels/screen-sharing/cc-screen-sharing-panel.c (+315/-0) panels/screen-sharing/cc-screen-sharing-panel.h (+29/-0) panels/screen-sharing/screen-sharing-module.c (+41/-0) panels/screen-sharing/screen-sharing-panel.ui (+400/-0) panels/screen-sharing/unity-screen-sharing-panel.desktop.in.in (+13/-0) panels/screen-sharing/vino-message-box.c (+156/-0) panels/screen-sharing/vino-message-box.h (+59/-0) panels/screen-sharing/vino-radio-button.c (+181/-0) panels/screen-sharing/vino-radio-button.h (+29/-0) po/POTFILES.in (+4/-0) |
| To merge this branch: | bzr merge lp:~robert-ancell/unity-control-center/screen-sharing |
| Related bugs: |
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| Matthew Paul Thomas (community) | design | 2014-07-15 | Needs Fixing on 2015-07-03 |
| Sebastien Bacher | 2014-07-15 | Approve on 2014-07-16 | |
| PS Jenkins bot (community) | continuous-integration | Approve on 2014-07-15 | |
|
Review via email:
|
|||
Commit Message
Add screen sharing panel to replace vino-preferences which is not available in Vino >= 3.10
| Robert Ancell (robert-ancell) wrote : | # |
| PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:12786
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
| Sebastien Bacher (seb128) wrote : | # |
Thanks Robert, is that code copied from vino or is that the one used in newer g-c-c for their privacy controls?
I've no reviewed the changeset yet, but having it listed even when vino is not installed seems suboptimal. We could make u-c-c depends on vino but it could be that some corporate installations want to uninstall vnc...
| Robert Ancell (robert-ancell) wrote : | # |
It's the code copied from Vino. The new g-c-c panel combines a number of sharing options that aren't applicable for us as well as the modern g-c-c style that doesn't really fit into u-c-c.
Agreed always showing is suboptimal. If Vino is not installed it just displays "Screen sharing support not installed". I tried to make it hide the panel if Vino is not available but that doesn't seem possible. I also considered adding an install button that would install the packages (first waiting to see if others think this direction is the right one).
The other option is to keep the panel in the Vino package as a big patch, but I think that will be more work over putting it in u-c-c.
| Sebastien Bacher (seb128) wrote : | # |
The code changes look fine to me, I'm unsure the feature is important enough to justify it's own settings panel like that though ... let's wait for mpt to comment on that before merging it in
| Matthew Paul Thomas (mpt) wrote : | # |
Is it possible to review this change on Ubuntu 14.04? autogen.sh gives me errors "No package 'gsettings-
(If you link screenshots, please show the variations for when Vino both is and isn't installed.)
| Robert Ancell (robert-ancell) wrote : | # |
Currently the system settings does not show any desktop sharing options [1]. Instead, you need to run the settings application [2] from the dash [3].
The proposal is to add a new panel to the system settings [4] [5]. If vino is no installed then this panel describes that [6].
[1] https:/
[2] https:/
[3] https:/
[4] https:/
[5] https:/
[6] https:/
| Robert Ancell (robert-ancell) wrote : | # |
We really should land this to unblock the vino update.
| Sebastien Bacher (seb128) wrote : | # |
is there anything in the vino update that makes us want to update? when I looked previously it was basically bringing nothing more that the simplifcation
| Robert Ancell (robert-ancell) wrote : | # |
It's a version that's supported upstream.
It has build fixes and removes old API usages which are likely to get harder to build against in the future.
It means we can drop a lot of patches we are carrying.
It means we are in sync with Debian.
It means it is easier to update when newer versions come out.
It means we're not blocking other users of Vino (e.g. Ubuntu GNOME).
What reason do we have not to update?
| Iain Lane (laney) wrote : | # |
The work's done, this MP is approved - why don't you get it in?
| Sebastien Bacher (seb128) wrote : | # |
Well, the main motivation to not update is that we don't need to atm, the current version builds&works fine and there is no much work/cool new features we/GNOME remix are missing, and that the panel feels like a regression compared to the standalone preferences
I guess we could decide the advantages you listed outweight the cost of the user experience regression though, if that's how you feel I can you can go ahead and land the changes
I would still appreciate a review from mpt there before we land it, having a "sharing" panel the way GNOME has it would be ok since it wouldn't be as specific, having a panel only for "screen sharing" feels like putting that feature a bit much in front to me...
| Matthew Paul Thomas (mpt) wrote : | # |
The Gnome System Settings designers seem to be in a period of confusion about whether to copy OS X or to copy iOS. The "Sharing" settings panel is the worst of both: combining settings for sharing wildly unrelated things in a single panel apparently because OS X does, and presenting them using a needlessly modal navigation list apparently because iOS does. I think it would be an extremely poor design to introduce to Ubuntu.
Sebastien is right that having a "Desktop Sharing" panel would be making that feature unusually prominent. Ideally I think it would be a tab of the "Displays" panel. But if the choice is between "Desktop Sharing" and the Gnome "Sharing" panel, I would strongly prefer the "Desktop Sharing" panel, renamed to "Screen Sharing" since it will eventually work on phones and tablets too.
"Desktop sharing support not installed" is an unhelpful message. I suggest instead something like "You need to install extra software to share your screen.", followed by a centered "Install Software…" button that invokes SessionInstaller to actually install it.
- 12787. By Robert Ancell on 2015-07-05
-
Merge with trunk
| Amr Ibrahim (amribrahim1987) wrote : | # |
Is this done for xenial?
| Jeremy Bicha (jbicha) wrote : | # |


Matthew - can you review for design? This adds a new panel "Screen Sharing" into the personal category.
Upsides:
- Feature is now more discoverable - previously you'd have to search in the dash for it
Downsides:
- Another panel
- Panel shows even if Vino is uninstalled - does this matter?