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

Aligned BoxLayout #1

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Aligned BoxLayout #1

wants to merge 5 commits into from

Conversation

fpabl0
Copy link
Member

@fpabl0 fpabl0 commented Feb 26, 2021

The aim of this proposal is to add cross axis alignment options for boxLayouts (HBox and VBox) that would allow them to be able to organize widgets with different cross axis sizes. Expandable feature and then the addition of a Expanded container is optional. The file aligned_boxlayout.md contains more detailed information about this proposal.

…extBaselineable and Expander object and removing old approach for expanded aligned boxes, added demo screenshot
@fpabl0
Copy link
Member Author

fpabl0 commented Apr 17, 2021

@andydotxyz I have updated the proposal. Unfortunately, I have realized we cannot split the expandable part from this. Because using container.NewBorder to expand objects would not align them. I have proposed a layout.NewExpander under the Architecture section. There, I explain better why container.NewBorder would not work very well with this addition.

I have updated the implementation too (it is a new link added in this proposal), now it effectively implement the baseline alignment for the cross axis :). I need to polish somethings, but in essence is that. You can see a screenshot of the mini demo I created in this document.

@fpabl0 fpabl0 changed the title Aligned and Expandable BoxLayout Aligned BoxLayout Apr 17, 2021
@fpabl0
Copy link
Member Author

fpabl0 commented Apr 23, 2021

@andydotxyz
From the call today, in order to remove the expand feature from this proposal, there is a need to add alignment capabilities to other layouts or a new layout. Current layouts are:

  • Center => does not need alignment, as it centers objects in both axis.
  • Form => does not need alignment, as it has a specific purpose.
  • MaxLayout => does not need alignment, as it sets all children to the full size (without available space, alignment is not possible).
  • PaddedLayout => same as MaxLayout.
  • GridWrap => could not have alignment, because it sets all children to a common cell size (without available space, alignment is not possible), and it automatically arrange objects in both axis depending on the available size and the number of objects.
  • Grid => same as GridWrap
  • Box => Could have alignment (this proposal), but this would break its definition:
      // For a VBoxLayout this will pack objects into a single column where each item
      // is full width but the height is the minimum required.
  • Border => Not sure if this layout should have alignment, because currently it does what it should do. Adding alignment would mean one more parameter to the container.NewBorder(...) that would be undesirable in my opinion, and it would mean to have alignment in both axis (but the only really needed would be horizontal Baseline alignment).

Note:
Using container.Border just to expand objects between others in just one axis does not feel natural, and have some limitations. The following layout could be hard to do it using Border (I don't want to say impossible because maybe I am not doing it well. I have tried many combinations, but I am not be able to build it with Border layout):

demo_2

Basically, if you have 3 objects and you want to expand just two of them (one should remain at its min size), how could it be accomplished currently?

1. First proposal:

  • Delete layout.Expander from here.
  • Break BoxLayout definition, as it won't take the whole cross axis size always (because now it would depend on the alignment).
  • Have something like container.NewBorderBaseline that only left and right objects with the middle object baseline (maybe this part will be confusing).

2. Second proposal:
Leave the proposal as it is now, but instead of modifying existing layouts, introduce a new one that could be a little "smarter" than the current BoxLayouts (basically adding the Layout.Expander).
The problem with this is that now it would be confusing, because BoxLayout and the new one would behave almost the same, with the only difference on the cross axis alignment and the Expander.

@andydotxyz
Copy link
Member

  • GridWrap => could not have alignment, because it sets all children to a common cell size (without available space, alignment is not possible), and it automatically arrange objects in both axis depending on the available size and the number of objects.
  • Grid => same as GridWrap

I don't agree that these cannot have alignment. Yes each cell is a common size, but the layout can arrange it's children as appropriate.
You have indicated that the expanding widget is required to do alignment in Box, but that in Grid this expansion makes alignment impossible. This seems to conflict and is very confusing.

@andydotxyz
Copy link
Member

Basically, if you have 3 objects and you want to expand just two of them (one should remain at its min size), how could it be accomplished currently?

1. First proposal:

  • Delete layout.Expander from here.
  • Break BoxLayout definition, as it won't take the whole cross axis size always (because now it would depend on the alignment).
  • Have something like container.NewBorderBaseline that only left and right objects with the middle object baseline (maybe this part will be confusing).

2. Second proposal:
Leave the proposal as it is now, but instead of modifying existing layouts, introduce a new one that could be a little "smarter" than the current BoxLayouts (basically adding the Layout.Expander).
The problem with this is that now it would be confusing, because BoxLayout and the new one would behave almost the same, with the only difference on the cross axis alignment and the Expander.

Here I am confused again - you pose the problem of how to have 2 expanded items in a layout, but both proposals talk about alignment. These need to be addressed separately, as has been discussed on two calls now.

I agree that the arrangement:

|              expanded             |min|             expanded                |

is not currently possible with standard layouts. So we could solve that with one proposal.

I do not agree that adding baseline alignment relies on the above, and I also don't agree that it is exclusive to the Box layout. We will code like that for the arrangement of proper rich text as part of Text Refactor as well.

We cannot continue to pursue these as part of the same proposal - the requirements are very blurred and it is impossible to truly understand the ramifications of such changes.

@fpabl0
Copy link
Member Author

fpabl0 commented Apr 27, 2021

Yes each cell is a common size, but the layout can arrange it's children as appropriate.

Yes, but the alignment in a Grid is different from a Box. In boxes, you are aligning the children against one axis. However, in Grids (if they have alignment), they need to arrange their children in both axis. So how user should specify the alignment?

GridWithColumns(3, ...)

      Col 1                  Col2                  Col3
-------------------------------------------------------------
|   Child 1         |    Child 2        |    Child 3        |      Row1
-------------------------------------------------------------
|   Child 4         |    Child 5        |    Child 6        |      Row2
-------------------------------------------------------------
|   Child 7         |    Child 8        |    Child 9        |      Row3
-------------------------------------------------------------

What is the main axis here? What should be the cross axis?

Main Axis should have alignment too?
If cross axis is the horizontal one, then the alignment should be applied per row?
GridWithRows(3, ...)

      Col 1                  Col2                  Col3
-------------------------------------------------------------
|   Child 1         |    Child 4        |    Child 7        |      Row1
-------------------------------------------------------------
|   Child 2         |    Child 5        |    Child 8        |      Row2
-------------------------------------------------------------
|   Child 3         |    Child 6        |    Child 9        |      Row3
-------------------------------------------------------------

Same here, what should the main axis and the cross axis?

@fpabl0
Copy link
Member Author

fpabl0 commented Apr 27, 2021

the requirements are very blurred and it is impossible to truly understand the ramifications of such changes.

Agree, but now I am lost, I don't know how to propose alignment for other layouts (specially because of the reasons I mentioned above). Could you give me ideas of how it should look like for other layouts?

@andydotxyz
Copy link
Member

Agree, but now I am lost, I don't know how to propose alignment for other layouts (specially because of the reasons I mentioned above). Could you give me ideas of how it should look like for other layouts?

By reducing to a clear scope I think that it is easy to answer. If the issue is around adding baseline alignment then it is quite clear how a grid would apply that - all items on a row would want have vertical alignment behaviour like the box discussion earlier.

@andydotxyz
Copy link
Member

Same here, what should the main axis and the cross axis?

This is a good question, the naming does not make it clear. If we consider vertical vs horizontal axis then it is pretty straight forward to discuss.

@fpabl0
Copy link
Member Author

fpabl0 commented Apr 28, 2021

If the issue is around adding baseline alignment then it is quite clear how a grid would apply that - all items on a row would want have vertical alignment behaviour like the box discussion earlier.

So GridWithColumns and GridWithRows would have only vertical alignment, while in Boxes, VBox would have horizontal alignment and HBox vertical alignment?

New constructors for Grid container should be then:

GridWithColumnsAligned(verticalAlign, cols, ...objects)
GridWithRowsAligned(verticalAlign, rows, ...objects)

?

@fpabl0
Copy link
Member Author

fpabl0 commented Apr 28, 2021

If we consider vertical vs horizontal axis then it is pretty straight forward to discuss.

Ok then CrossAlignment type should be only Alignment?

@andydotxyz
Copy link
Member

So GridWithColumns and GridWithRows would have only vertical alignment, while in Boxes, VBox would have horizontal alignment and HBox vertical alignment?

That isn't what was in my mind. If you are looking to address vertical and horizontal alignment then grid would support both, whereas Box has one (or the other, depending on vertical vs horizontal).

It probably comes down to what types of alignment this is trying to address, baseline only operates horizontally of course.

@fpabl0
Copy link
Member Author

fpabl0 commented Apr 28, 2021

If you are looking to address vertical and horizontal alignment then grid would support both

If this is done, Grid definition is not longer true as it would not stretch its children to the cell size in any axis. Is that ok?
Also, having the two alignments in Grid means new constructors like:

NewGridWithColumnsAligned(horizontalAlignment, verticalAlignment, cols, ...objects)
NewGridWithRowsAligned(horizontalAlignment, verticalAlignment, rows, ...objects)

They don't seem like great constructors 😕, any ideas what it should look like?

It probably comes down to what types of alignment this is trying to address, baseline only operates horizontally of course.

Well, originally this was just for BoxLayout and for it CrossAlignment was ideally, but now I am not sure. Maybe Alignment is the best name now?

@andydotxyz
Copy link
Member

If this is done, Grid definition is not longer true as it would not stretch its children to the cell size in any axis.

Modifications to Grid would be no bigger than Box - both would have to reduce the space available to force alignment.

Comparing the horizontal box and a row of a grid you can see that for vertical placement they are the same (except the box picks the least space that all widgets require, whereas grid uses a portion of available). Both algorithms expand and would need to be adjusted to support alignment because currently they position uniformly to the same height.

@fpabl0
Copy link
Member Author

fpabl0 commented Apr 29, 2021

Comparing the horizontal box and a row of a grid you can see that for vertical placement they are the same (except the box picks the least space that all widgets require, whereas grid uses a portion of available).

Got it. So Grids and Boxes are going to be the only layouts impacted by the alignment?

@andydotxyz
Copy link
Member

Got it. So Grids and Boxes are going to be the only layouts impacted by the alignment?

That's bit what I was meaning - the grid wrap and border also have items that pack items next to each other in a row that could require alignmment.
Center, Max and Padded show items over each other, it could be argued that alignment does not make sense, or you could argue that it still makes sense because items over each other may wish to be aligned.
Form has it's own approach - but you could probably argue that each row of a form should be aligned in some way that could be controlled by the layout.

Basically if you start looking to add alignment into layout algorithms it pretty much hits every one of them.
This is one of the reasons that I was suggesting we consider standard widget sizes instead, and look into what sort of items may not fit this model. We may discover that rich text is a special case and should be addressed as a separate issue (such as the ongoing Text Refactor work).

@fpabl0
Copy link
Member Author

fpabl0 commented May 1, 2021

That's bit what I was meaning - the grid wrap and border also have items that pack items next to each other in a row that could require alignmment.

As I said before, I think Border has a specific purpose and it already does its job (the problem is -IMHO- that it is overused for other things). GridWrap should have alignment only if we decide other Grids would have it too.

Center, Max and Padded ... it still makes sense because items over each other may wish to be aligned.

I don't think those layouts should have alignment. They already do their jobs.

Basically if you start looking to add alignment into layout algorithms it pretty much hits every one of them.

Yes, that's why I proposed this as a feature for only BoxLayouts (specially because they are the only layouts that does not expand by default).

This is one of the reasons that I was suggesting we consider standard widget sizes instead, and look into what sort of items may not fit this model.

Standard widget sizes should remain as they are now (unless they have some problems like the button and the entry iirc), however limiting everything to that size, you are limiting also the appearance for custom widgets.

We may discover that rich text is a special case and should be addressed as a separate issue (such as the ongoing Text Refactor work).

Yes, absolutely that is one of the widgets that need baseline alignment, but it could use it internally without problems (I mean it wouldn't depend on this directly).

@andydotxyz
Copy link
Member

Yes, that's why I proposed this as a feature for only BoxLayouts (specially because they are the only layouts that does not expand by default).

I return to my thought that alignment and expansion should be separated. Box would have to take more space to align, but would no longer expand the child elements. This result is the same as a layout that expands but would require each element take a little less space to be able to align. The outcome is gaps in what was otherwise a uniform fill.

@fpabl0
Copy link
Member Author

fpabl0 commented May 5, 2021

Box would have to take more space to align, but would no longer expand the child elements. This result is the same as a layout that expands but would require each element take a little less space to be able to align. The outcome is gaps in what was otherwise a uniform fill.

Sorry, I don't understand what are you trying to say.

@andydotxyz
Copy link
Member

Sorry, I don't understand what are you trying to say.

I am trying to find new illustrations to show that expansion and alignment are separate issues.

@fpabl0
Copy link
Member Author

fpabl0 commented May 5, 2021

I am trying to find new illustrations to show that expansion and alignment are separate issues.

I am aware they are separate issues, but because the Expanded objects and Non-Expanded objects are commonly used together, they cannot be worked separately IMHO. Most of the popular cross platform frameworks for developing apps have mixed the alignment with the expansion feature in their layouts: Flutter, Ionic, React, etc.

I understand your point, but I think that way would limit other kind of alignments and doesn't have clean way to expand objects in Boxes while aligning them too.

So not sure about the future of this proposal, because we have two possibilities with problems:

  1. Mix Alignment and Expand feature in BoxLayout, but it would break the current layout definitions and add a new canvas object that would be only useful for Boxes.
  2. Add only alignment to layouts, but as you noted it could lead to add alignment to every single layout and make containers constructors overloaded with new parameters (in my opinion), that I think is undesirable.

@andydotxyz
Copy link
Member

It was discussed on the call again today, it was generally agreed that by standardising widget definition to include a standard baseline we can solve many of the common use-cases without any new API.
We will clearly ensure that RichText manages baseline appropriately across the various text sizes/fonts etc for internal consistency.

Therefore we suggested leaving this proposal to one side and may revisit it after 2.1 and the various text enhancements to see what still needs to be addressed.

@fpabl0
Copy link
Member Author

fpabl0 commented May 15, 2021

It was discussed on the call again today, it was generally agreed that by standardising widget definition to include a standard baseline we can solve many of the common use-cases without any new API.

That's fine. But just to clarify, (as I said before) this proposal is not against the standardization. The only goal of this proposal is to be able to align widgets with different sizes (mainly heights) to have widgets like the one discussed in fyne-io/fyne#1701 but as you pointed me out in that moment it is not recommended currently because Fyne does not have a way to align widgets in the cross axis, so this proposal wanted to solve that.

We will clearly ensure that RichText manages baseline appropriately across the various text sizes/fonts etc for internal consistency.

Great! I am very excited about that proposal 🤩. But I would like (if it is possible) that the function in charge for measuring the distance to the baseline is public (if it is implemented).

@andydotxyz
Copy link
Member

The only goal of this proposal is to be able to align widgets with different sizes (mainly heights) to have widgets like the one discussed in fyne-io/fyne#1701

That one issue can be resolved in the form layout.

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