Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add paddings to Stack and create Layout function #19

Merged
merged 1 commit into from
Jun 26, 2014
Merged

Add paddings to Stack and create Layout function #19

merged 1 commit into from
Jun 26, 2014

Conversation

boppreh
Copy link
Contributor

@boppreh boppreh commented Jun 23, 2014

I tried to make an intuitive function to quickly create good-looking layouts. I liked the result, but I understand if you don't want to increase the API size right now, so this is only a suggestion. Anyway, the padding/margin code may be useful by itself.

This change is twofold:

  • First, add unexported attributes padding and marginBorder to Stack that work as expected. I'm open to name suggestions here.
  • Second, export a function Layout that creates a stack with automatic orientation, padding and border.

Layout works by giving only the topmost stacks a margin, and settings its orientation to "vertical". All nested stacks inside it are oriented perpendicular to its parent and have only padding, not margin. As a bonus, any nil value is converted to a stretchy space.

So for example, this code:

w := ui.NewWindow("Test Window", 400, 250)
w.Open(ui.Layout(ui.NewLabel("Task Description"),
    ui.NewProgressBar(),
    nil,
    ui.Layout(ui.NewButton("Button 1"),
        ui.NewLineEdit("Edit"),
        nil,
        ui.Layout(ui.NewButton("Button 2"),
            ui.NewButton("Button 3"),
            ui.NewButton("Button 4")),
        ui.Layout(ui.NewButton("Button 5"),
            nil,
            ui.NewButton("Button 6")))))

Creates this layout:

goui

I personally liked the results and will be using the function, merging or not. I'm also considering making a Stretch function, so you can write ui.Layout(button1, ui.Stretch(button2), button3) but right now I can't think of an elegant implementation because the stretching information is on the parent.

@andlabs
Copy link
Owner

andlabs commented Jun 23, 2014

Padding and spacing is something I've been thinking about, yes. First, I wanted to make some changes to Label about vertical text alignment, then I was thinking of just having padding on/off, choosing reasonable pixel values for the given platform.

As for Layout: I actually wanted to recreate GtkGrid (more info) but as a subpackage, since Stack and Grid provide the necessary functionality. However, this looks useful as it is, so I would just need to go through your function (though I might drop it in a subpackage). I could have both facilities with a subpackage =P

I'll go through everything once I resume work on the project (probably later today or tomorrow or in the next few days)

@andlabs
Copy link
Owner

andlabs commented Jun 25, 2014

reminder to self: should nil in the stack/grid constructors also insert a space?

@andlabs
Copy link
Owner

andlabs commented Jun 25, 2014

All right. I'll use this space to think about padding and margins. I'm not sure if I can take your implementation of it as is since this will be done on a platform dependent level but.

Windows
Guidelines. Note that these are in dialog units; conversion between dialog units and pixels is done in prefsize_windows.go.

  • 4 dialog units between controls
  • There's a suggestion of 7 dialog units between unrelated controls. I don't know if I can do that without having to ask programmers what constitute related and unrelated controls. Group boxes could help here, but they have their own margining rules.
  • 7 dialog units of margin on all sides

The killer here is "7 dialog units of margin on all sides". I'm going to guess that the dialog unit conversion will have to be done on the window, and then copied down to each individual control. Not a problem on the backend side (this is what hte implementation does, so =P ), but will require more parameter shuffilng than I just did.

GTK+
Guidelines. Page 2.

  • 12 pixels of window margin
  • 12 pixels horizontally and 6 pixels (I'm guessing based on other information) vertically between controls
  • 18 pixels between unrelated controls; see above
  • 6 pixels between icons and labels (will become important if and when I add icons)

Notice that GNOME suggests 6-pixel increments.

MAC OS X
This one's harder.

The Human Interface Guidelines say "Use the proper spacing between controls. When controls are spaced evenly in a window, the window is more attractive and easier for users to use. The layout guides in Interface Builder show you the recommended spacing between controls and between UI elements and window edges." In other words, since we're not using Interface Builder, we're screwed. If the spacing has /never/ changed and /will never change/, then we don't really have a problem, but...

There is Auto Layout, but again, the values to use are hidden from view; I would need to create Auto Layout constraints for everything and that'll be too much work to duplicate work I'm already doing!

The Auto Layout guide does hint that the standard spacing between controls is 12 pixels. I'm currently waiting in IRC for more information.

@boppreh
Copy link
Contributor Author

boppreh commented Jun 25, 2014

Thanks, I had been looking for that Windows guidelines for a while. I could only find their Metro style guide.

I'll try adding system-dependent margins and padding as a start, and later give a shot at grouping controls. I think this is might be doable in a transparent fashion for the library user.

@andlabs
Copy link
Owner

andlabs commented Jun 25, 2014

All right, so.

I tried starting to write in all the prerequisite work to use the above information. I realized that this would require yet another massive change to the whole sizing and layout system. And since the proper positioning of labels is already invasive enough as it is...

I'm strongly leaning toward scrapping the whole sizing system and redoing that all from scratch. Stack and Grid themselves would need only minimal change. but the actual backend sizing stuff will disappear.

But I'm not exactly sure as to what...

Package ui works in quadrant IV coordinates, but some calculations become extremely difficult on Mac OS X without requiring a bidirectional control tree since Mac OS X uses quadrant I coordinates. Right now I compensate by passing a winheight variable. However, the proper positioning of labels depends on the distance from the bottom left corner of a neighboring control to the baseline of the first line of text. And not the bottom left corner of that control's frame, but rather of its alignment rectangle, which is something else different. Since we only started getting programmatic access to this information in Mac OS X 10.7, this dashes all hopes of ever reintroducing 10.6 compatibility in the future.

On the Windows side, I have a giant mess and the need to manually compute dialog box coordinates (or just treat everything as a dialog box) and text widths. But other than that, there are no issues. Windows uses quadrant IV coordinates.

On GTK+ I don't have any real issues. Labels have full control over their horizontal and vertical position, so I can just have labels position themselves properly. GTK+ uses qudrant IV coordinates.

I'll probably need to model all this in pseudocode.

Your Layout() function is surprisingly smaller than I thought it would be, though I see you actually touch the Stacks directly. I could possibly just have that put in as is, but I'll do that after the margin/padding change.

@andlabs
Copy link
Owner

andlabs commented Jun 26, 2014

What do you think of this?

initial pseudocode fefc060#diff-777c16bc56e4fc4518379caa1f5f7200
mostly complete final code that will be dropped in https://github.com/andlabs/ui/blob/master/newsizing

@boppreh
Copy link
Contributor Author

boppreh commented Jun 26, 2014

Looks sound to me, I have only two comments to make:

  • Do you think the neighbor detection will work for nested layout controls? I didn't get this impression, but I could be wrong. For example in my code snippet up there, remembering Layout just creates nested Stacks. Detecting neighbors in that case may be a hard problem.
  • Shouldn't the OS X quadrant conversion happen on a lower level, just when you are about to draw the control? Currently doResize receives "inverted" coordinates depending on the system running, and this could considerably complicate the algorithm and introduce bugs.

I added a bounty to the stack overflow question, let's see if we can get that sorted out.

@andlabs
Copy link
Owner

andlabs commented Jun 26, 2014

For nested layouts, the intended result will be that the first deepest control of the sub-layout on the right is the neighbor of the first deepest control of the sub-layout on the left. I'm not sure how this will work in recursive calls, but it's worth trying. I'll make that clearer in the final code.

Actually, doResize() now is the last step in the resizing. I'll rename it to commitResize(). As I mentioned earlier, fine-tuning the allocated space has to be done on the transformed coordinates to make things easier when aligning label baselines with their neighboring controls on OS X (I tried to do these calculations on nontransformed coordinates this morning and just got confused).

Thanks for adding the bounty.

@andlabs
Copy link
Owner

andlabs commented Jun 26, 2014

I've pushed margin and padding code now. I unfortunately wasn't able to use your code (I wrote this offline) but it handles separate margin and padding in both directions. Turn it on for an entire window with w.SetSpaced(true) (I could change that name)

I would add the equivalent Grid code, but I can't seem to figure out if the margin/padding is different from what it is in Stack. What do you think?

diff --git a/grid.go b/grid.go
index 8a42bcf..b1c2a52 100644
--- a/grid.go
+++ b/grid.go
@@ -134,16 +134,19 @@ func (g *Grid) allocate(x int, y int, width int, height int, d *sysSizeData) (al

    var current *allocation     // for neighboring

+   // TODO return if nControls == 0?
    // before we do anything, steal the margin so nested Stacks/Grids don't double down
    xmargin := d.xmargin
    ymargin := d.ymargin
    d.xmargin = 0
    d.ymargin = 0
-   // 0) inset the available rect by the margins
+   // 0) inset the available rect by the margins and needed padding
    x += xmargin
    y += ymargin
    width -= xmargin * 2
    height -= ymargin * 2
+   width -= (len(g.colwidths) - 1) * d.xpadding
+   height -= (len(g.rowheights) - 1) * d.ypadding
    // 1) clear data structures
    for i := range g.rowheights {
        g.rowheights[i] = 0
@@ -197,15 +200,16 @@ func (g *Grid) allocate(x int, y int, width int, height int, d *sysSizeData) (al
                current = nil           // spaces don't have allocation data
            }
            allocations = append(allocations, as...)
-           x += g.colwidths[col]
+           x += g.colwidths[col] + d.xpadding
        }
        x = startx
-       y += g.rowheights[row]
+       y += g.rowheights[row] + d.ypadding
    }
    return
 }

 // filling and stretchy are ignored for preferred size calculation
+// We don't consider the margins here, but will need to if Window.SizeToFit() is ever made a thing.
 func (g *Grid) preferredSize(d *sysSizeData) (width int, height int) {
    max := func(a int, b int) int {
        if a > b {
@@ -214,6 +218,8 @@ func (g *Grid) preferredSize(d *sysSizeData) (width int, height int) {
        return b
    }

+   width -= (len(g.colwidths) - 1) * d.xpadding
+   height -= (len(g.rowheights) - 1) * d.ypadding
    // 1) clear data structures
    for i := range g.rowheights {
        g.rowheights[i] = 0

If you resend the Layout pull request I can merge that.

andlabs added a commit that referenced this pull request Jun 26, 2014
Add a Layout function for higher-level layouts. I might put this in a subrepository. Thanks @boppreh
@andlabs andlabs merged commit b398150 into andlabs:master Jun 26, 2014
@andlabs
Copy link
Owner

andlabs commented Jun 26, 2014

I wrote a test and it seems the Grid spacing is the same, but I'm still not sure about the test program... what do you think anyway?

Also, what do you think of the system-provided padding values? (Compared to your first screenshot...)

@boppreh
Copy link
Contributor Author

boppreh commented Jun 26, 2014

Wait, I didn't finish fixing layout.go! The way you implemented the padding, it's passed by the allocate function. The way I had done was that each stack had a padding and margin, specified during creation.

The problem now is that for the Layout function to give different margins to different stack it'll have to create a new structure that handles allocate differently based on its depth (i.e. only the first structure uses margin). It may take a few minutes for me to fix this.

@andlabs
Copy link
Owner

andlabs commented Jun 26, 2014

Oh sorry :( It shouldn't be too hard to fix...

@boppreh
Copy link
Contributor Author

boppreh commented Jun 26, 2014

Yeah, not that big of a problem to fix, but right now I think the project is not building because of this.

@andlabs
Copy link
Owner

andlabs commented Jun 26, 2014

What build issue are you getting?

@boppreh
Copy link
Contributor Author

boppreh commented Jun 26, 2014

The layout.go as you merged was referencing nonexistent attributes. I've added a new pull request to fix the issue: #21

@andlabs
Copy link
Owner

andlabs commented Jun 26, 2014

Done. Thanks for all the waiting and trouble ^^'

What exactly do you think of what I have now visually, though (in comparison to the values you used to make your original screenshot)? Just curious

@boppreh
Copy link
Contributor Author

boppreh commented Jun 26, 2014

Those were just random values, no specific reason for them. I'm in Ubuntu now and the spacing looks good. I'll try Windows later today.

I just found weird that the background of the test application is black in Ubuntu. I don't know what color it should be, but I'm sure it's not black. I can open a new Issue if you prefer.

screenshot from 2014-06-26 17 07 35

@andlabs
Copy link
Owner

andlabs commented Jun 26, 2014

Ah, I thought that would happen eventually. Will fix, especially now that the KDE bug has been fixed.

andlabs added a commit that referenced this pull request Jun 26, 2014
…his breaks the Ubuntu GTK+ 3 themes (Ambianace and Radiance) with their correct renderer (see #19 (comment)), and the KDE bug has now been fixed.
@andlabs
Copy link
Owner

andlabs commented Aug 18, 2014

Update: the rewrite gets rid of the black background issue on Ubuntu. I'm not sure what caused it, but my custom GTK+ container doesn't trip anything, so...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants