[SuperEditor][SuperReader][SuperTextField] Merge stylesheet styles with default text style. (Resolves #2727)

[angelosilvestre]

[SuperEditor][SuperReader][SuperTextField] Merge stylesheet styles with default text style. (Resolves #2727)

Currently, SuperEditor, SuperReader and SuperTextField don't honor the enclosing DefaultTextStyle, which mean we cannot override the font family globally.

This PR adds that ability.

EDIT: This was made as an opt-in feature for all widgets to avoid silently changing the styles for people that are relying on the currently no-inheriting behavior.

[angelosilvestre]

EDIT: I made it opt-in for SuperTextField also.

@matthew-carroll It seems some SuperTextField tests failed, so maybe we should indeed make this opt-in in SuperTextField also? If not, we would need to override the default text style of the existing tests.

[matthew-carroll]

I'm wondering if this is too low level of a location to instruct the method to inherit the default text style. I know we provide a BuildContext but I'm wondering if it makes sense for this generic extension to have a concept of inheriting a text style. Would it make more sense for the caller to pass that in?

[angelosilvestre]

It makes sense. Updated it.

[matthew-carroll]

Can you add a test that rebuilds the tree with a different style and makes sure that the new value is used after the pump?

Please do the same for SuperReader and SuperTextField.

[angelosilvestre]

I already added a "changes visual text when the enclosing default text style changes" to each of them.

[matthew-carroll]

Please add a constructor Dart Doc that explains what should be passed for defaultTextStyle and what happens if nothing is passed for it.

[angelosilvestre]

Updated.

[matthew-carroll]

Instead of adding this conditional logic, can we create the layout presenter with a null default text style and then update it here without any conditional at all?

[angelosilvestre]

I added this logic because there are some situations where we re-create the presenter inside didUpdateWidget. For example, when the stylesheet changes.

It we always create the presenter with a null default textstyle, then we need to replicate the code that sets the default text style everywhere we create the presenter.

[matthew-carroll]

I didn't follow that explanation. Can you provide an example of what you mean?

[angelosilvestre]

For example, if we want to create the presenter with a null textStyle, we'd need to update the following places to also set the defaultTextStyle after creating it.

https://github.com/superlistapp/super_editor/blob/933863b6963dc7ccb81eb20d3fe97a2729d8f2ce/super_editor/lib/src/default_editor/super_editor.dart#L492-L504

https://github.com/superlistapp/super_editor/blob/933863b6963dc7ccb81eb20d3fe97a2729d8f2ce/super_editor/lib/src/default_editor/super_editor.dart#L505-L512

[matthew-carroll]

The first code snippet you included doesn't seem to reference the layout presenter at all. The second code snippet calls a method to create the layout presenter. Neither of these code snippets appear to show code that would need to be changed?

It would be great if you could show exactly what code snippet exists now, and exactly what code snippet it would need to become, so that we can avoid any confusion about what you're trying to say.

[angelosilvestre]

Both snippets call _createLayoutPresenter. Each one would need to change to the following:

_createLayoutPresenter();
_docStylesheetStyler.defaultTextStyle = DefaultTextStyle.of(context).style;

[matthew-carroll]

Let's try to never say "merged" because it always begs the question as to which style wins out.

[angelosilvestre]

Updated.