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

Improve ergonomics for single-section Texts #14854

Closed
Olle-Lukowski opened this issue Aug 21, 2024 · 11 comments
Closed

Improve ergonomics for single-section Texts #14854

Olle-Lukowski opened this issue Aug 21, 2024 · 11 comments
Labels
A-Text Rendering and layout for characters A-UI Graphical user interfaces, styles, layouts, and widgets C-Feature A new feature, making something new possible P-High This is particularly urgent, and deserves immediate attention S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it!
Milestone

Comments

@Olle-Lukowski
Copy link
Contributor

What problem does this solve or what need does it fill?

With the changes introduced in the PR linked below, the common case for Text where there is only one TextSection gets a bit more annoying. This should be fixed ASAP, in any case, before 0.15.

Info

Old API:

commands.spawn(TextBundle::from_section(...));

Current API that will need improvements:

commands.spawn(TextBundle::default()).with_child(TextSection::new(...));

What solution would you like?

I think the easiest option is to make add a Commands::spawn_text() method, this is simply a shorthand for spawning one TextSection as a child of a Text.

What alternative(s) have you considered?

There are probably alternatives, and we should discuss those

Additional context

This is important once #14572 gets merged.

@Olle-Lukowski Olle-Lukowski added C-Feature A new feature, making something new possible A-UI Graphical user interfaces, styles, layouts, and widgets P-High This is particularly urgent, and deserves immediate attention A-Text Rendering and layout for characters labels Aug 21, 2024
@Olle-Lukowski Olle-Lukowski added this to the 0.15 milestone Aug 21, 2024
@Olle-Lukowski Olle-Lukowski changed the title Improve ergonomics for single-sections Texts Improve ergonomics for single-section Texts Aug 21, 2024
@rparrett
Copy link
Contributor

Some additional background:

  • A previous incarnation of Improve ergonomics for single-section Texts #14854 added functionality for a single text section to be spawned alongside Text.

    commands.spawn((TextBundle::default(), TextSection::new("")));

    But this seemed controversial due to being potentially confusing / footgunny, and it added some complication to text systems, user code for looking up a text section entity from a cosmic-text Buffer, etc. So it was punted.

  • Maybe bsn will save us in time.

    But the examples in the discussion above assume the ability to create a single-section Text entity without any hierarchy.

    With hierarchy, bsn is just as clunky.

    bsn! {
      Text [
          Section("Hello World")
      ]
    }
  • with_child is new since Add with_child to simplify spawning when there will only be one child #14594 and was added specifically to make this a bit less gross. Before:

    commands
        .spawn(TextBundle::default())
        .with_children(|parent| {
            parent.spawn(TextSection::from("Hello"));
        });

@cart
Copy link
Member

cart commented Aug 22, 2024

@NotAFile
Copy link
Contributor

I see another three options:

  • Address the common case (especially in examples) of wanting some simple debug text which is not intended to be pretty seperately using a slimmed-down text component with a single span and limited styling. This could be a DebugText or perhaps something gizmo-like.
  • I don't like the idea of having to remember to use a separate command to spawn text vs everything else. A simple way around that would be a SystemParam, which mirrors what is often done in the ecosystem and not that different to a beginner.
  • The other way around it would be a more generic version of spawning an entity with a child. This could be as complex as "required parents" (mirroring required components, although a macro for it wouldn't be necessary for now) or as simple as a new spawn method that allows specifying a parent and child in the same function call, perhaps even something like spawn_with_default_parent.

@rparrett
Copy link
Contributor

rparrett commented Aug 24, 2024

To briefly summarize the linked discord conversation above, Cart is proposing essentially combining the concepts of Text and TextSection and allowing them to be nested.
edit: I was confused, see next messages below.

This is a bit of a departure from the current minimal state of #14572, and #14791 is a prerequisite, though one could possibly imagine doing something similar with bundles.

But it seems that this is something we don't want to put off for later, so it is possible that this issue is redundant.

// 1. [Hello]
commands.spawn(Text::new("Hello"));

// 2. [Hello, World]
commands
  .spawn(Text::default())
  .with_children(|parent| {
    parent.spawn(Text::new("Hello"));
    parent.spawn(Text::new("World"));
  });

// 3. [Hello, World]
commands
  .spawn((Node, Style { Display::Flow, ..default() }))
  .with_children(|parent| {
    parent.spawn(Text::new("Hello"));
    parent.spawn(Text::new("World"));
  });

// 4. [Hello, World]
commands
  .spawn(Text::new("Hello"))
  .with_child(Text::new("World"));

// 5. [H, ello, W, orld]
commands
  .spawn(Text::new("H"))
  .with_children(|parent| {
    parent.spawn(Text::new("ello"));
    parent.spawn(Text::new("W"))
      .with_child(Text::new("orld"));
  });

@cart
Copy link
Member

cart commented Aug 24, 2024

In that conversation we reached consensus that the last two cases above shouldn't behave that way (implicitly "flattening" the text layout in nested Text hierarchies).

Instead, the rule would simply be: by default, treat each Text as a separate / autonomous text section, laid out according to how it / its parent haven chosen. If a parent specifies Display::Flow, then all of its children will be laid out as if they are a group of back-to-back text sections.

@cart
Copy link
Member

cart commented Aug 24, 2024

Pulling in @viridia, who was part of this conversation on discord.

@viridia
Copy link
Contributor

viridia commented Aug 24, 2024

Thanks for pinging me. Here's a more formal specification of what I proposed:

  1. Canonically, text nodes are leaves of the tree. Any children of a text node are ignored by the layout.
  2. The layout of a text node depends on the parent node's Display style.
  3. For text nodes whose parent is Display::Grid, Display::Flex, or Display::Block, each text node is treated as a separate row/column/cell; the cosmic-text layout algorithm is performed on that single entity. There can be word wrap, but only if the text entity won't fit in the allotted space.
  4. For text nodes which are children of a parent with Display::Flow (a new display type), all text and non-text children are word-wrapped together; adjacent text nodes are merged (which means ligatures can cross node boundaries). Non-text items are treated as unitary, non-breakable glyphs for layout purposes. A single cosmic-text layout context is allocated for the parent.
  5. Cosmic-text layout contexts will be provided implicitly by the Bevy engine, users do not need to explicitly insert them as components. (How this is to be implemented is TBD).

This proposal is explicitly modeled after the behavior of HTML/SGML text blocks, but with significant simplifications (e.g. we don't strip leading and trailing spaces, the "display" property only affects internal, not external, layout behavior, etc. The Display::Flow display type corresponds to the internal layout rules of CSS display: inline.

@viridia
Copy link
Contributor

viridia commented Aug 24, 2024

As mentioned in the discord, the goal here is not to regurgitate the web or allow building of a web browser within Bevy; the goal is to permit document-like UIs such as the character bios or quest logs seen in games like Witcher 3 or Jedi: Last Survivor.

There is also the issue of styling, which is part of the problem of text-section ergonomics, but might need to be a separate issue. Individual text nodes can have styles, but there's no internal layout, so properties like display::Grid or gap are meaningless. However, style properties such as left or margin are external layout properties, and thus are meaningful, but might have different effects in the context of a Display::Flow parent. For text nodes in a grid/flex/block context, these layout parameters are applied to the bounding box of the text entity. For text nodes in a flow context, it's "whatever the closest equivalent that cosmic-text supports", otherwise we punt (in the short term anyway).

As some of you know, I am a fan of inherited styles, however there's a number of prominent users on Discord who don't like them / find them confusing. The compromise that I have implemented in Quill is to allow inheritance for only those styles which are text specific, of which there are just 5: font, size, weight, style, decoration. Other kinds of inheritance can be done in other ways, using reactive contexts or whatever. Being able to assign a text style to a parent and have that style apply to all of the individual text sections is very convenient and ergonomic - for example, my Button widget imposes a style for its children (which of course individual sections can override, but in practice never do). Style inheritance is implemented via a separate system - it requires a marker component to be inserted into the text node - that is independent from Quill.

@UkoeHB
Copy link
Contributor

UkoeHB commented Sep 3, 2024

See discussion #15014

@eidloi
Copy link

eidloi commented Sep 27, 2024

building more complex UI structures has been explored deeply by sickle_ui, and I think it would make sense to have a generic way to make widgets (like the UiBuilder it has) instead of specifically for text.

@BenjaminBrienen BenjaminBrienen added the S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it! label Sep 28, 2024
@alice-i-cecile
Copy link
Member

Completed in #15591.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Text Rendering and layout for characters A-UI Graphical user interfaces, styles, layouts, and widgets C-Feature A new feature, making something new possible P-High This is particularly urgent, and deserves immediate attention S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it!
Projects
None yet
Development

No branches or pull requests

9 participants