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

Implementing a rich-segmented text view widget #2074

Closed
diamondburned opened this issue Mar 9, 2021 · 17 comments
Closed

Implementing a rich-segmented text view widget #2074

diamondburned opened this issue Mar 9, 2021 · 17 comments
Labels
duplicate This issue or pull request already exists

Comments

@diamondburned
Copy link

diamondburned commented Mar 9, 2021

Preamble

UI frameworks should have a comprehensive API to allow users style different segments of texts as well as inserting images or even allowing absolute positioning of arbitrary widgets. A use case for this would be formatting a chat message to have rich formatting.

An example of a UI framework that does this is GTK3: the GtkTextView widget allows styling segments of texts as well as inserting arbitrary widgets (!). This flexibility not only allows font sizes, colors and other attributes to be modified dynamically over certain regions of texts, but it also allows inserting inline images, similar to how inline emojis can be implemented in HTML.

As of right now, the only widget that can display some kind of text styling is the Label widget, but the widget does not allow dynamic typing over different segments of texts.

Existing Issue

Issue #149 proposes a widget for previewing Markdown for a rich text widget. There are several issues with having a widget just for Markdown, but the most obvious one would be the limitation to only Markdown and not any other arbitrary markup formats. Instead, it would be much better to have a widget that allows flexible text segmenting and styling, and then to write a separate Markdown-to-segments converter.

Proposal

Part 1: Existing Modifications

It would be nice for fyne.TextStyle to be appended to be richer and express more of what Markdown has to offer:

type TextStyle struct {
	Bold          bool
	Italic        bool
	Monospace     bool
	Underline     bool // new
	Strikethrough bool // new
    InvertColors  bool // new
}

This isn't particularly within the scope of this proposal, but it would make the "rich" text much richer.

Part 2: Package richtext

The new richtext package proposed will provide a multi-line text widget capable of displaying a large block of text with segments attributed to bounds within the text region. The base for the texts and its segments should be:

package richtext

// Rich describes a piece of rich text styled using the list of text segments.
type Rich struct {
    Text     string
    Segments []Segment
}

// Segment describes a generic text segment. A valid segment would be any
// segment in this package that implements the Segment interface.
type Segment interface {
    Bounds() (start, end int)
}

// ThemedSegment describes a text segment that overrides the parent segment
// or widget's styling. A zero-value should be returned for methods that
// don't override the particular style given in its arguments.
type ThemedSegment interface {
    Segment

    // These methods are taken from fyne.Theme. They're probably better off
    // being split out from fyne.Theme to be embedded in here.

	Color(fyne.ThemeColorName, fyne.ThemeVariant) color.Color
	Font(fyne.TextStyle) fyne.Resource
}

// WidgetSegment describes a widget segment to be inserted into the preset
// location.
type WidgetSegment interface {
    Segment
    CreateRenderer() fyne.WidgetRenderer
}

The segment interfaces are intentionally left as interfaces instead of structs to allow the caller to arbitrarily implement their own dynamic styling segments. The API might incur too much boilerplate on the caller, however, and interface-to-interface assertions tend to be slow. A struct version of these segments may work out:

// Segment describes a generic text segment. A valid segment would be any
// segment in this package that implements the Segment interface.
type Segment interface {
    Bounds() (start, end int)
    segment()
}

// ColorKey describes the set of color attributes to look up.
type ColorKey struct {
    Name    fyne.ThemeColorName
    Variant fyne.ThemeVariant
}

// ThemedSegment describes a text segment that overrides the parent segment
// or widget's styling. Any key not present in the map will imply that its
// value should be inherited.
type ThemedSegment interface {
    Start, End int
    Colors map[fyne.ColorKey]color.Color
    Fonts  map[fyne.TextStyle]Resource
}

func (tseg ThemedSegment) Bounds() (int, int) { return tseg.Start, tseg.End }
func (tseg ThemedSegment) segment()           {}

// WidgetSegment describes a widget segment to be inserted into the preset
// location.
type WidgetSegment struct {
    Location int
    Renderer fyne.WidgetRenderer
}

func (wseg WidgetSegment) Bounds() (int, int) { return wseg.Location, wseg.Location }
func (wseg WidgetSegment) segment()           {}

The struct idea seems to be much more sound; it also seems more straightforward to use and would take less boilerplate. The map constructs inside ThemedSegment are slightly weird, however.

Note that this is merely an initial draft, and that suggestions and improvements are very welcomed. I've also considered adding fyne.TextAlign into the Segment, but how it would work exactly, I wouldn't have anything for reference.

Appendix

None of the changes proposed above should be breaking.

As of right now, it seems that Fyne does not yet hyphenate or ellipsize broken or overflowing words. It would be nice in the future to include toggling them when they do get added.

See #1660.

Reference

This API was largely based off of my text segment API for a chat protocol. The reference documentation of its text segment API can be found here. This file contains a simple implementation of a segment that changes the color of a segment of text.

Not all of these APIs will translate to Fyne, however. Fyne is a UI framework/library, so its APIs should ideally be tied down to the core styling, while something like cchat is a chat library, so its segments would describe a more abstract style of text.

@andydotxyz
Copy link
Member

Thanks for this, lots of good thoughts. I'm not sure that I would agree with the addition of TextStyle.InvertColors bool // new however, as the style does not relate to colour, and inverting colours would be better suited alongside where colours are specified.

Overall the concept of segments is clearly required, but it would better match the Fyne semantic API if they were defined with meaning instead of theme attributes. For example Segment.Type = important would be far preferrable to ThemableSegment.Color = black, ThemableSegment.Style = bold for example.
Taking a semantic approach allows the theme to provide style across the app consistently instead of the text API taking control.

Perhaps you can expand on what the struct based illustration has segment() for? They seem to be no-op in the illustration...

As of right now, it seems that Fyne does not yet hyphenate or ellipsize broken or overflowing words.

Indeed - the intention is that the wrapping code should include it. Personally I would prefer that the hypen/ellipsis is always on, but if it is to be controlled it could be either through the Wrapping or the TextStyle, whichever fits better. See #1660

@diamondburned
Copy link
Author

diamondburned commented Mar 9, 2021

Perhaps you can expand on what the struct based illustration has segment() for? They seem to be no-op in the illustration...

The segment() method is used to restrict the possible implementations of the interface. Asserting to structs that aren't made by the package and would therefore be unable to implement the unexported method segment() would cause a compile-time error when asserting. The type assertion code for bounds would be something like this:

switch bounds := bounds.(type) {
case ThemedSegment:
case WidgetSegment:
case string: // does not compile!
}

Overall the concept of segments is clearly required, but it would better match the Fyne semantic API if they were defined with meaning instead of theme attributes. For example Segment.Type = important would be far preferrable to ThemableSegment.Color = black, ThemableSegment.Style = bold for example.

I think that if we don't expose an API that allows changing attributes arbitrarily and restrict the API to a preset of attributes, then that would be very bad; it would restrict the user heavily in what they can do.

With the interface API, something like this can still probably be implemented and used on the interfaces. Otherwise, I think a theme constructor to construct a prestyled segment would work. But I don't think the API should be limited to just preset styles.

@andydotxyz
Copy link
Member

But I don't think the API should be limited to just preset styles.

As an API beased on semantics and intent there would need to be a really strong reason to expose explicit style manipulation.
Perhaps look at TextGrid - it provides some standard styles along with the ability to provide custom ones.
The text grid is not ideal, but it feels closer to our API design to provide the semantics and let developers extend to provide specifics.

@diamondburned
Copy link
Author

But I don't think the API should be limited to just preset styles.

As an API beased on semantics and intent there would need to be a really strong reason to expose explicit style manipulation.
Perhaps look at TextGrid - it provides some standard styles along with the ability to provide custom ones.
The text grid is not ideal, but it feels closer to our API design to provide the semantics and let developers extend to provide specifics.

The flexibility to do any styling is a reason to expose explicit style manipulation. I'm not sure what you mean. Why would you want to restrict the user over exposing a more customizable and flexible API? I'm not sure what that would help accomplish.

Not to mention, the goal of this API would be to expose a low-level flexible widget that other widgets can be based on in the first place, so why restrict them to something less?

From what I can see of TextGrid, TextGridStyle is practically the same as ThemedSegment, only much less flexible. At least, the lack of font style in TextGrid is understandable, but not allowing that degree of flexibility in this API would restrict the user a lot in what they can do.

@andydotxyz
Copy link
Member

The flexibility to do any styling is a reason to expose explicit style manipulation. I'm not sure what you mean. Why would you want to restrict the user over exposing a more customizable and flexible API? I'm not sure what that would help accomplish.

I am trying to keep us in-line with the semantic widget API that is styled by the theme. And so "heading" is the semantic portion and "larger text, bold" is in the theme.

Not to mention, the goal of this API would be to expose a low-level flexible widget that other widgets can be based on in the first place.

We don't really have low-level widgets, so the path is potentially unclear. Maybe this is not truly a Widget, but feels too complex to truly be part of the canvas package as well.

The TextGrid is aiming for more semantic helpers, such as standard types to avoid apps specifying colours directly for most cases. When an app specifies a colour it needs to test against all the possible theme combinations. If we can provide the theme capabilities so that the widget can just say "keyword" or "error" without having to wory about bad colour combinations or updating on theme change etc.

@diamondburned
Copy link
Author

We don't really have low-level widgets, so the path is potentially unclear. Maybe this is not truly a Widget, but feels too complex to truly be part of the canvas package as well.

For the record, I am proposing a new package richtext, because I'm well aware that everything will be pretty chunky and can't really be self-contained. I think another TextView widget could be added into package widget that uses this widget for its implementations but enforces the consistent themes and styles.

When an app specifies a colour it needs to test against all the possible theme combinations.

Not necessarily. I don't think the library should enforce this assumption onto others. It strictly limits the usage of such a thing.

@andydotxyz
Copy link
Member

When an app specifies a colour it needs to test against all the possible theme combinations.

Not necessarily.

This is only not true if an app completely replaces the theme.
Maybe I don’t understand what this “not necessarily” is implying. It sounds like you think that testing light/dark/user primary colour chose can be ignored but I don’t understand how.

@diamondburned
Copy link
Author

diamondburned commented Mar 12, 2021

This is only not true if an app completely replaces the theme.
Maybe I don’t understand what this “not necessarily” is implying. It sounds like you think that testing light/dark/user primary colour chose can be ignored but I don’t understand how.

Whether or not the user needs to choose a color depending on the theme should be up to the user; it shouldn't be forced that the user must do it that way. There are cases where the user may receive raw hex colors and would prefer the exact colors to be used in place rather than having the library try to be smarter than it should be.

I do agree that there should be an API that handles this, but it should absolutely not be the only API available. In other words, the entire point of my proposal is to have an API that delegates the choices to the user, not the library. The library can (and should) have higher-level APIs that make certain choices for the user, but it should absolutely not force the user in a certain way.

@andydotxyz
Copy link
Member

The approach we have taken is that the standard widgets will do standard things, and can be extended to customise. I think that fits in this case as well.

@diamondburned
Copy link
Author

The approach we have taken is that the standard widgets will do standard things, and can be extended to customise. I think that fits in this case as well.

If you start with a restrictive widget that's not nearly as flexible as this, then how can they be "extended" to be as customizable?

@andydotxyz
Copy link
Member

Well, I suppose that is the purpose of a design exercise.
It is much easier to keep the toolkit manageable if every widget provides minimal API and supports extending.
From there we can add functionality as required, when it seems we missed important base features.

@diamondburned
Copy link
Author

Well, I suppose that is the purpose of a design exercise.
It is much easier to keep the toolkit manageable if every widget provides minimal API and supports extending.
From there we can add functionality as required, when it seems we missed important base features.

I think that being simple shouldn't mean to limit what the end user can do. As I see it, the current proposal is simple: it provides a bare-bone way to style texts without any concern regarding the current theme, leaving full control to the user while being easier to implement (there are only primitive styling to be implemented).

This freedom from simplicity would also allow extending the widget to make higher-level ones, such as widgets that have predefined style-sets for Markdown and such.

@andydotxyz
Copy link
Member

it provides a bare-bone way to style texts without any concern regarding the current theme

Then it does not fit in the widget package - these are behaviour based APIs for managing standard widgets according to the current theme.

This freedom from simplicity would also allow extending the widget to make higher-level ones, such as widgets that have predefined style-sets for Markdown and such.

What I am trying to say is that the opposite is possible. Basic widgets provide basic APIs, with the ability to extend to add custom behavior. Apps using the basic widgets should look great no matter what. In this regard only errors should be red (for example) and primary text will always match the user's default theme (unless the app provides an alternative theme).
This is the design of the widget package and the overall mantra of the project.

Perhaps this relates to #1741 - we are trying to deffine a colour pallete based on semantics rather than values, which will lead to a much better match of TextGrid to the theme approach described above.

@diamondburned
Copy link
Author

Then it does not fit in the widget package - these are behaviour based APIs for managing standard widgets according to the current theme.

I'm not proposing a new widget in the widget package. I'm proposing a new package. This would indeed not be suitable for that package.

Basic widgets provide basic APIs, with the ability to extend to add custom behavior.

I fail to see how this would be carried out if the "basic API" locks the user to only a very limited set of styles.

@andydotxyz
Copy link
Member

Hopefully something like the following would support semantic basis for the main API whilst remaining extensible:

var (
	RichTextStyleInline RichTextStyle
	RichTextStyleParagraph RichTextStyle
	RichTextStyleHeading RichTextStyle
	RichTextStyleSubHeading RichTextStyle
	RichTextStyleQuote RichTextStyle
	RichTextStyleURL RichTextURL
	RichTextStyleEmphasis RichTextStyle
)

type RichText struct {
	BaseWidget

	Segments []TextSegment
}

type TextSegment struct {
	Text string
	Type RichTextStyle
}

type RichTextStyle interface {
	Alignment() fyne.TextAlign
	Inline()    bool
	TextSize()  float32
	TextStyle() fyne.TextStyle

	Segments() []TextSegment
}

type RichTextURL interface {
	RichTextStyle
	URL() url.URL
}

@andydotxyz
Copy link
Member

Main discussion is happening on Text Refactor and further discussion of the rich text aspects at #21

@andydotxyz andydotxyz added the duplicate This issue or pull request already exists label Apr 28, 2021
@andydotxyz
Copy link
Member

Closing this to focus discussions on the items above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
duplicate This issue or pull request already exists
Projects
None yet
Development

No branches or pull requests

2 participants