After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 733367 - Put wizard & properties in a dialog
Put wizard & properties in a dialog
Status: RESOLVED FIXED
Product: gnome-boxes
Classification: Applications
Component: wizard
unspecified
Other Linux
: Normal normal
: 3.22
Assigned To: GNOME Boxes maintainer(s)
GNOME Boxes maintainer(s)
Depends on:
Blocks: 733368
 
 
Reported: 2014-07-18 15:47 UTC by Zeeshan Ali
Modified: 2016-03-31 13:22 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
libvirt-machine-props: Make dialogs modal (2.15 KB, patch)
2014-08-29 00:53 UTC, Zeeshan Ali
none Details | Review
wizard: Make page setter public (841 bytes, patch)
2014-08-29 00:53 UTC, Zeeshan Ali
committed Details | Review
Put wizard in a modal dialog (18.51 KB, patch)
2014-08-29 00:53 UTC, Zeeshan Ali
none Details | Review
Drop now redundant WizardToolbar (8.23 KB, patch)
2014-08-29 00:53 UTC, Zeeshan Ali
none Details | Review
wizard: Remove now redundant cancel_button field (756 bytes, patch)
2014-08-29 00:53 UTC, Zeeshan Ali
none Details | Review
Put properties in a modal dialog (16.80 KB, patch)
2014-08-29 00:53 UTC, Zeeshan Ali
none Details | Review
Drop now redundant PropertiesToolbar (6.13 KB, patch)
2014-08-29 00:53 UTC, Zeeshan Ali
none Details | Review
properties-sidebar: Adjust spacing & margins (1.26 KB, patch)
2014-08-29 00:53 UTC, Zeeshan Ali
none Details | Review
libvirt-machine-props: troubleshoot transient over props dialog (1.32 KB, patch)
2014-08-29 00:53 UTC, Zeeshan Ali
none Details | Review
Drop now redundant Sidebar class (3.46 KB, patch)
2014-08-29 00:53 UTC, Zeeshan Ali
committed Details | Review
alignment to the guides (50.53 KB, image/png)
2014-10-22 18:42 UTC, Jakub Steiner
  Details

Description Zeeshan Ali 2014-07-18 15:47:47 UTC
Was talking to Allan just now and he reminded me how the wizard's topbar buttons are very hard to reach and even hard to notice on a large screen. People have noticed this before. Allan suggested that we put wizard in a dialog so its as small as it needs to be.
Comment 1 Zeeshan Ali 2014-08-27 16:25:09 UTC
Also it would be good to then put properties in dialog at the same time. I think jimmac also suggested something like that. I already have wip patches in wip/wizard-n-props-in-dialog2 that implements this but there are some minor issues to be resolved and then the task of trying to divide this into smaller patches.
Comment 2 Zeeshan Ali 2014-08-29 00:53:03 UTC
Created attachment 284767 [details] [review]
libvirt-machine-props: Make dialogs modal

Make dialogs launched from properties, modal. Almost all dialogs should
be modal anyway and in this case, it's certainly true.

At the same time make "Save" dialog transient over "Troubleshooting log"
dialog as its launched by that.
Comment 3 Zeeshan Ali 2014-08-29 00:53:07 UTC
Created attachment 284768 [details] [review]
wizard: Make page setter public

In a following patch, we'll introduce a new widget that will need to
take charge of switching wizard pages.
Comment 4 Zeeshan Ali 2014-08-29 00:53:12 UTC
Created attachment 284769 [details] [review]
Put wizard in a modal dialog

With some UI transitions removed during the port away from clutter, its
better to have it in a dialog. This also helps reduce the size of wizard
to buttons on both sides are not so far away from each other and hence
easy to reach.

Known issue: Wizard dialog changes size for some reason when arriving to
review page.
Comment 5 Zeeshan Ali 2014-08-29 00:53:16 UTC
Created attachment 284770 [details] [review]
Drop now redundant WizardToolbar
Comment 6 Zeeshan Ali 2014-08-29 00:53:21 UTC
Created attachment 284771 [details] [review]
wizard: Remove now redundant cancel_button field
Comment 7 Zeeshan Ali 2014-08-29 00:53:25 UTC
Created attachment 284772 [details] [review]
Put properties in a modal dialog

With some UI transitions removed during the port away from clutter, its
better to have it in a dialog.

Known issues:

* This implies that now we can't change name of machine through title
  anymore. Main reason is that there seems to be no way to set a custom
  widget as title of a dialog but I don't think many will miss this
  feature anyway, especially now that its a dialog.

* Page titles are missing in the sidebar when coming to properties from
  wizard and hence the ability to switch pages.
Comment 8 Zeeshan Ali 2014-08-29 00:53:28 UTC
Created attachment 284773 [details] [review]
Drop now redundant PropertiesToolbar
Comment 9 Zeeshan Ali 2014-08-29 00:53:32 UTC
Created attachment 284774 [details] [review]
properties-sidebar: Adjust spacing & margins

After getting moved into the new properties dialog, we need to adjust
some spacing and margins to look good.
Comment 10 Zeeshan Ali 2014-08-29 00:53:35 UTC
Created attachment 284775 [details] [review]
libvirt-machine-props: troubleshoot transient over props dialog

"Troubleshooting log" dialog should now be transient over the new
properties dialog rather than app window.
Comment 11 Zeeshan Ali 2014-08-29 00:53:38 UTC
Created attachment 284776 [details] [review]
Drop now redundant Sidebar class
Comment 12 Zeeshan Ali 2014-08-29 13:45:55 UTC
jimmac tried the patches and provided some input: https://mail.gnome.org/archives/release-team/2014-August/msg00089.html (see also following mails in the thread).
Comment 13 Jakub Steiner 2014-10-22 12:53:31 UTC
I've updated the wires[1] to feature a major omission from the existing properties - box renaming.

[1] https://raw.githubusercontent.com/gnome-design-team/gnome-mockups/master/boxes/wires/properties-modal.png
Comment 14 Lasse Schuirmann 2014-10-22 13:18:34 UTC
(In reply to comment #13)
> I've updated the wires[1] to feature a major omission from the existing
> properties - box renaming.
> 
> [1]
> https://raw.githubusercontent.com/gnome-design-team/gnome-mockups/master/boxes/wires/properties-modal.png

Cool, thanks!

Some feedback:
1. Tabs
I assume it is due to the draft nature of this document that these tabs in the dialog don't use the whole width like all other GNOME apps now do?

2. Alignment
Especially on the devices tab (which is wrong highlighted in the mockup btw, confused me a bit) I think it would be good to make the width of all the control elements the same, align them to each other.

We also probably want the labels right-aligned as for
https://developer.gnome.org/hig/stable/visual-layout.html.en
and without a colon. (This also applies to the Display tab)

3. Snapshots
3.1. Current
Since we have a new design anyway, I'd like to take the opportunity and restate my idea from https://bugzilla.gnome.org/show_bug.cgi?id=735687#c2 :
This design implies to the user that the current state _is_ equivalent to the state of the last snapshot. This is not true since the current state is a derivative of the most recent snapshot. So we could better visualise this in the following way IMO:

[ ] Old Snapshot
 |
[ ] Most Recent Snapshot
 |
[+] Current State

This way we have the indication that the current state is not the most recent snapshot, we show very well that you can turn the Current State into a snapshot (and then you get a new current state at least if the VM runs) and it doesnt take more space than it does right now.

This makes the whole thing way more intuitive and correct IMO.

3.2. Spacing
I think there's too much whitespace between the box-diagram thing and the snapshot labels. This looks very weird so I'd either
a) Put them directly beneath the snapshots.
If you get the non-linear expansion you'd have to shift all labels a bit to the right.
b) Maybe center the labels?
Not sure but that way its clearer that the label belongs to both whats left and whats right of it.
c) Put the Icon that suggests you could change settings of a snapshot (the gear symbol) on the left side too.
They all belong to each other. Why seperate the poor souls?

3.3. The gear symbol
Isnt really intuitive IMO. Though I don't know something better right now.
Comment 15 Lasse Schuirmann 2014-10-22 13:25:15 UTC
And maybe it would make sense to add a copy to clipboard button for the troubleshooting log because thats what you probably want if you're in trouble.
Comment 16 Jakub Steiner 2014-10-22 13:36:26 UTC
Some excellent notes. Updated the wires.
Comment 17 Lasse Schuirmann 2014-10-22 13:48:52 UTC
Devices tab still needs alignment and display tab looks a bit weird now to me somehow. Probably because the center of alignment is at such a strange position.
Comment 18 Jakub Steiner 2014-10-22 18:42:10 UTC
I think the reason why you say you have a problem with devices tab alignment is the indentation due to the list container perhaps? Attaching a screenshot of the guides.

As for the display tab, left aligned labels give a better anchor, perhaps.
Comment 19 Jakub Steiner 2014-10-22 18:42:54 UTC
Created attachment 289165 [details]
alignment to the guides
Comment 20 Lasse Schuirmann 2014-10-23 06:24:40 UTC
(In reply to comment #19)
> Created an attachment (id=289165) [details]
> alignment to the guides

Thanks for explaining.

It's no question that it technically makes sense to align these things this way. What I meant was rather:
1. I would bump up the width of the select button to match the width of the toggles

2. I would align the buttons to each other - independently from the container. (Labels as well.) Is there anything about those cases in the HIG? I think it would look way better.
Comment 21 Jakub Steiner 2014-10-23 11:29:05 UTC
While I don't disagree very strongly, I think keeping the different levels of indentation helps achieving structure. The auto-forward switch has a higher level of importance than the individual devices. 

I would agree visually the strict grid layout is more aesthetically pleasing. Similarly you are right about the button sizes, but considering translations and no way of fixing width with gtk we can really only rely on the right alignments for the buttons.
Comment 22 Zeeshan Ali 2014-11-25 15:18:39 UTC
So I improved and changed most of the patches and pushed them, with biggest changes being:

1. Properties and wizard are now in a GtkWindow, rather than GtkDialog (so that we have better control over the topbar). However, they are still transient and behave and look very much like dialogs.
2. Sidebar is gone. Instead all sidebar controls have an inview replacement.
3. While sizes can still use some improvments, wizard and properties look a lot better now.

These are the patches that were pushed but not attached here:

commit: e008d2fa075af33e773d308bcff54c2120a9cd6c

    properties: Update page names
    
    This should have been part of 91b4cc22 and 77538e5.

commit: 4abde9e1b6e667a6d7293f7b6a9ee521e78853bd

    Put wizard in a separate window
    
    With some UI transitions removed during the port away from clutter, its
    better to have it in separate window. This also helps reduce the size of
    wizard to buttons on both sides are not so far away from each other and
    hence easy to reach.
    
    Known issue:
    
    * Wizard window takes a wide space and changes size when arriving to
      review page. This is solved in a following patch when we adjust the
      size and contents of 'no kvm' warning infobar.
    
    * This breaks properties access from wizard's review page since
      properties view is still inside the AppWindow. This is solved when we
      move properties to its own window as well in a following patch.
    
    * Notifications are appearing on the AppWindow and since this window is
      modal, there is no way to interact with them right now. Even though in
      wizard, we don't show any actionable notification, we really need to
      solve this soon.

commit: 2c10b9f73f6b7faa727ee0fa9e9e38f1387b0663

    topbar: Remove wizard_toolbar property
    
    Wizard is now in its own window where it has a dedicated topbar of its
    own so WizardToolbar is not needed in main window's topbar anymore.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=733367

commit: 700afafa5cc6938e3b053f82c71d0537367f4131

    wizard: Make review page fit in smaller space
    
    One of the points of putting the wizard into a separate window is that
    it would then take less screen space. Without these adjustments, wizard
    takes a lot of horizontal space.

commit: c5b0291103f7986afbb871f1769caff8d43007b2

    wizard: Specify step number in title
   
commit: 117b118710db1c00abbf59247aacbd05cc4e0ae8

    Drop WizardSidebar
    
    Drop the sidebar from wizard. It looks weird now in the wizard window
    and what little functionality it provides has already been replaced by
    previous patches in this series.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=733367

commit: 6aaf52af4d96136282522b1518ad6aa1fa5e009e

    properties: Specify min content size on ScrolledWindow
    
    We are about to move propertiew view into its own window thats made
    transient over the main window. If minimum width and height are not set,
    ScrolledWindow will hide most of the content once that change is in
    place.
    
    So in this patch, we set minimum width to be 640 and minimum height to
    be 480. This should be harmless except for screens with extremely low
    resolution, in which case I'm sure this won't be our biggest worry
    anyway.

commit: ede964bfe49a63b272a880bf98cbfd714761204a

    Put properties in a separate window
    
    With some UI transitions removed during the port away from clutter, its
    better to have it in a separate window.
    
    Known issues:
    
    * Similar to WizardWindow, notifications are going into AppWindow.
    
    * Our snapshots UI relies on disabling all controls to get out of the
      snapshots view while an operation is in progress. We lose most of this
      ability to disable all controls with this patch. We really shouldn't be
      doing this anyway so lets actually solve this issue after these UI
      rework changes are merged into git master.
    
    * Troubleshooting dialog is not accessible unless you close properties
      window. Although the real solution would be to not launch dialogs from
      dialogs (or dialog-like windows), we'll solve this in a following
      patch by making troubleshooting dialog also modal.

commit: 6e03fa21082787b9da4adc944051b064b8e05642

    mini-graph: Adjust for bigger size
    
    We are about to put mini graphs into properties' main view (and then
    remove them from sidebar), where they'll need to be much bigger than
    they currently are.
    
    TODO: Make them look good on bigger size as well.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=733367

commit: cedb28a5bbf69411564193fd1b9553bd7cb0305b

    libvirt-machine-props: Troubleshoot dialog now modal
    
    "Troubleshooting log" dialog (and dialogs launched from it) should now be
    modal since otherwise users won't be able to interact with it since
    PropertiesWindow is modal.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=733367

commit: 4f15c5acd25e15e6efe19a5b70ca801c17fd2d73

    i-properties-provider: Add Property.flushed signal
    
    Add a signal to Property to notify interested parties that said property
    has been flushed.

commit: 37013275781d992ccee03ee59b6cdcc7526e73a1

    libvirt-machine-props: Add resource stats graphs
    
    Add resource stats graphs to properties under 'System' page.
    
    This is part of move of sidebar contents to main view before sidebar can
    be dropped.

commit: 3eb94a50a5c1785a15dfa2ca1e8d94272268415b

    libvirt-machine-props: Refactor system props code a little
    
    Put all the code to add troubleshoot log button and dialog into a
    separate method.
    
commit: 306827f97044a0cd12ff844de661fa45227162b8

    libvirt-machine-props: Change placement of troubleshoot log
    
    Put the troubleshoot log button between resource stats and resource
    controls, as per new UI mockups.

commit: 3aaf481eac666fb6a350cac5e09a3b0529bba6ab

    libvirt-machine-props: Add 'Force Shutdown' button
    
    Add'Force Shutdown' button to properties under 'System' page, next to
    'Troubleshooting log' button.
    
    This is part of move of sidebar contents to main view before sidebar can
    be dropped.
    
commit: 6a5b93e8cd607947e73193628235edf660a62b7b

    properties: Turn it into Gtk.Notebook from Gtk.Stack
    
    According to the new UI mockup, we need it to be a notebook so that
    we have page switching controls before we drop the sidebar.
    
commit: 03a22eac5936a437c35e4a1376c7a371cb2050cf

    Drop now redundant PropertiesSidebar
    
    Drop the sidebar from properties. It looks weird now in the separate
    (smaller) window and all the functionality it provides has already been
    replaced by previous patches to this.

Re-used patches:

Attachment 284768 [details] pushed as 8c377d0 - wizard: Make page setter public
Attachment 284776 [details] pushed as 31f3a9f - Drop now redundant Sidebar class