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

Margin model #329

Open
dhardy opened this issue Jul 1, 2022 · 3 comments
Open

Margin model #329

dhardy opened this issue Jul 1, 2022 · 3 comments

Comments

@dhardy
Copy link
Collaborator

dhardy commented Jul 1, 2022

KAS attempts to provide correct margins automatically. This appears to mostly work, but with some little issues.

Current approach

Exterior margins

Each element, as part of SizeRules, details the expected (minimum) margin on each side. When two items are placed next to each other, the margin between them must be at least as large as the margin expected from each item on the neighbouring side.

Alignment of elements excludes margins. This results in better visual alignment of rows/columns/grids but potentially larger than expected margins.

The same margin applies between an element and the edge of the window. Correctness of this choice is debatable. (This is relatable to the frame margins discussed below.)

Interior margins

Frames are a special case: they encapsulate some other element, and how content margins should be handled is less clear.

The FrameRules type is used to declare the size of a frame, exterior margin size, and the (minimum required) internal margin size. (An internal margin may be used e.g. to draw a selection box.)

How the margins of a FrameRules are combined with the margins from its content's SizeRules depends, currently on what type of frame is used, but correctness of this approach is highly questionable:

  • Frame styles InnerMargin, EditBox and MenuEntry (horizontal orientation only) use surround_with_margin, meaning content margin must appear inside the frame
  • Frame style NavFocus (used for navigation focus visual element only) uses surround_as_margin, meaning the frame itself is considered a margin around the content
  • Other frame styles (Frame, Popup, MenuEntry (vertical), Button) use surround_no_margin, meaning that the content's margin is simply ignored

There is logic to the above choices, though imperfect:

  • NavFocus should be as inconspicuous as possible when not used
  • Buttons have a large enough frame that extra margin is not normally required
  • EditBox requires internal margin since the (typical) text content requires this margin for drawing an edit cursor

Problems

Having three different modes of combining frame and content margins smells like bad desgin.

Further, it may be preferable to have the method used to combine margins be part of the FrameRules type instead of inferred from usage.

It is not currently possible for content to request a larger (or any) margin inside some frames, such as in a Button. This is notably problematic for ComboBox, which places an arrow mark inside a Button frame, and results in insufficient margin between this mark and the inside edge of the button frame.

It is currently tricky for UI designers to have any direct control over margins, though since our objective is "perfect margins automatically", it would be important to review such cases individually to see why additional control was required. (At the moment, probably the only solution would be to use a special wrapper widget replacing or extending content margins.)

Possible changes

We could always use surround_as_margin instead of surround_no_margin, reducing us to two types of margin combination. Practically there is not much difference since the frame is usually large enough to cover content's margin requirements already.

We could instead use surround_with_margin for some/all of the above frames. The main issue here is that buttons end up too large since the "chrome" of the button frame is already large, and get even larger when a content margin is used in addition.

(Further, pop-up menus have somewhat special layout which gets broken when using surround_with_margin, though arguably this special layout is just a poor hack).

Text elements could push the required text margin inside the text layout object. This would allow EditBox not to require an internal margin, but would make TextButtons larger and would affect visual alignment of text with other elements.

We could try special solutions to problematic cases, e.g. for ComboBox wrap content in a frame inside the button to force internal margins (horizontally).

We could try giving all elements two different margin sizes: that within frames and that between elements. This seems over complex.

@dhardy
Copy link
Collaborator Author

dhardy commented Aug 11, 2022

Problematic cases are theme dependent, e.g. (both using 150% zoom):
button-margins
The top row of buttons is drawn by FlatTheme: small, visually distinct borders, which appear too close to content. The second row is drawn by ShadedTheme: wide borders with no line of contrast at the inner edge of the border. Thus, the theme should have control over margin-squashing behaviour.

Possible solution: leave SizeRules as is, but change FrameRules such that the internal margin is content_margin.saturating_sub(implied_inner_margin). Then ShadedTheme's buttons may have a larger implied_inner_margin while FlatTheme has 0.

Also: do we need FrameRules::inner_margin?

@dhardy
Copy link
Collaborator Author

dhardy commented Aug 12, 2022

Consider a row of items: A has height 20 and margin 5, B has height 10 and margin 10 (both with a total height of 30). But what about the row?

  • If we naively combine values via max, the row has height 20 and margin 10 (total 40)
  • If A and B are both centre-aligned, both items can be placed with correct alignment and margin using a total height of 30
  • If A and B are both top-aligned, we require a top margin of 10, 20 for content, and a bottom margin of 5 (total 35)
  • If A is top-aligned and B is bottom-aligned, we require a top margin of 5, 20 for content and 10 for the bottom margin (total 35); effectively alignment of the largest item is ignored and the smallest item is aligned within the content allocation
  • The situation is the same if A is centred or stretched: only B's alignment matters
  • If B has stretch alignment, it gets height 20 and requires both margins to be 10 (total 40)

It is a little surprising that alignment affects the size requirement, and this turns out to be a problem: size requirements are calculated by Layout::size_rules, which does not know about alignment. There is some utility however: we can potentially remove the alignment code within several widgets' set_rect method. (Also, we can set text alignment in ThemeSize::text_rules.)

Proposal: add optional input alignment to AxisInfo; add mandatory output alignment to SizeRules. Remove alignment from set_rect.

@dhardy
Copy link
Collaborator Author

dhardy commented Aug 19, 2022

What should happen regarding margins on the edge of a window?
calculator
Notice that:

  • The background colour differs from the desktop theme — for now, any attempt to match this (other than with custom themes) is beyond the project scope.
  • An alternative approach would be to let the window draw its own decorations; this is currently also beyond scope.
  • We don't necessarily need any margin between the buttons or text box and the window decorations, though this may depend on the decorations.

For now I won't make any changes here, but I wonder if (sometimes) subtracting margins at the edge of the window would make sense. This would be possible by changing the frame model introduced by #348 to include a subtractive component as suggested above replacing the current inner margin.

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

No branches or pull requests

1 participant