Skip to content

Commit

Permalink
Cleanup the text widgets to not be generic (#260)
Browse files Browse the repository at this point in the history
This is fallout from #241. As discussed on Zulip, using different
underlying text types here is a very advanced feature, which will not be
needed by any real consumers. It also creates some footguns

Probably conflicts with #257, so that should land first
  • Loading branch information
DJMcNab authored May 5, 2024
1 parent 2c1fa8a commit 9542aaf
Show file tree
Hide file tree
Showing 12 changed files with 116 additions and 91 deletions.
4 changes: 2 additions & 2 deletions crates/masonry/examples/calc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -255,8 +255,8 @@ impl AppDriver for CalcState {
.get_element()
.child_mut(1)
.unwrap()
.downcast::<Label<String>>()
.set_text((&*self.value).into());
.downcast::<Label>()
.set_text(&*self.value);
}
}

Expand Down
22 changes: 11 additions & 11 deletions crates/masonry/src/widget/button.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use crate::paint_scene_helpers::{fill_lin_gradient, stroke, UnitPoint};
use crate::text2::TextStorage;
use crate::widget::{Label, WidgetMut, WidgetPod, WidgetRef};
use crate::{
theme, AccessCtx, AccessEvent, BoxConstraints, EventCtx, Insets, LayoutCtx, LifeCycle,
theme, AccessCtx, AccessEvent, ArcStr, BoxConstraints, EventCtx, Insets, LayoutCtx, LifeCycle,
LifeCycleCtx, PaintCtx, PointerEvent, Size, StatusChange, TextEvent, Widget,
};

Expand All @@ -25,11 +25,11 @@ const LABEL_INSETS: Insets = Insets::uniform_xy(8., 2.);
/// A button with a text label.
///
/// Emits [`Action::ButtonPressed`] when pressed.
pub struct Button<T: TextStorage> {
label: WidgetPod<Label<T>>,
pub struct Button {
label: WidgetPod<Label>,
}

impl<T: TextStorage> Button<T> {
impl Button {
/// Create a new button with a text label.
///
/// # Examples
Expand All @@ -39,7 +39,7 @@ impl<T: TextStorage> Button<T> {
///
/// let button = Button::new("Increment");
/// ```
pub fn new(text: T) -> Button<T> {
pub fn new(text: impl Into<ArcStr>) -> Button {
Button::from_label(Label::new(text))
}

Expand All @@ -54,25 +54,25 @@ impl<T: TextStorage> Button<T> {
/// let label = Label::new("Increment").with_text_brush(Color::rgb(0.5, 0.5, 0.5));
/// let button = Button::from_label(label);
/// ```
pub fn from_label(label: Label<T>) -> Button<T> {
pub fn from_label(label: Label) -> Button {
Button {
label: WidgetPod::new(label),
}
}
}

impl<T: TextStorage> WidgetMut<'_, Button<T>> {
impl WidgetMut<'_, Button> {
/// Set the text.
pub fn set_text(&mut self, new_text: T) {
pub fn set_text(&mut self, new_text: impl Into<ArcStr>) {
self.label_mut().set_text(new_text);
}

pub fn label_mut(&mut self) -> WidgetMut<'_, Label<T>> {
pub fn label_mut(&mut self) -> WidgetMut<'_, Label> {
self.ctx.get_mut(&mut self.widget.label)
}
}

impl<T: TextStorage> Widget for Button<T> {
impl Widget for Button {
fn on_pointer_event(&mut self, ctx: &mut EventCtx, event: &PointerEvent) {
match event {
PointerEvent::PointerDown(_, _) => {
Expand Down Expand Up @@ -261,7 +261,7 @@ mod tests {
let mut harness = TestHarness::create_with_size(button, Size::new(50.0, 50.0));

harness.edit_root_widget(|mut button| {
let mut button = button.downcast::<Button<&'static str>>();
let mut button = button.downcast::<Button>();
button.set_text("The quick brown fox jumps over the lazy dog");

let mut label = button.label_mut();
Expand Down
24 changes: 13 additions & 11 deletions crates/masonry/src/widget/checkbox.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,46 +20,48 @@ use crate::{
};

/// A checkbox that can be toggled.
pub struct Checkbox<T: TextStorage = ArcStr> {
pub struct Checkbox {
checked: bool,
label: WidgetPod<Label<T>>,
label: WidgetPod<Label>,
}

impl<T: TextStorage> Checkbox<T> {
impl Checkbox {
/// Create a new `Checkbox` with a text label.
pub fn new(checked: bool, text: T) -> Checkbox<T> {
pub fn new(checked: bool, text: impl Into<ArcStr>) -> Checkbox {
Checkbox {
checked,
label: WidgetPod::new(Label::new(text)),
}
}

/// Create a new `Checkbox` with the given label.
pub fn from_label(checked: bool, label: Label<T>) -> Checkbox<T> {
pub fn from_label(checked: bool, label: Label) -> Checkbox {
Checkbox {
checked,
label: WidgetPod::new(label),
}
}
}

impl<T: TextStorage> WidgetMut<'_, Checkbox<T>> {
impl WidgetMut<'_, Checkbox> {
pub fn set_checked(&mut self, checked: bool) {
self.widget.checked = checked;
self.ctx.request_paint();
}

/// Set the text.
pub fn set_text(&mut self, new_text: T) {
///
/// We enforce this to be an `ArcStr` to make the allocation explicit.
pub fn set_text(&mut self, new_text: ArcStr) {
self.label_mut().set_text(new_text);
}

pub fn label_mut(&mut self) -> WidgetMut<'_, Label<T>> {
pub fn label_mut(&mut self) -> WidgetMut<'_, Label> {
self.ctx.get_mut(&mut self.widget.label)
}
}

impl<T: TextStorage> Widget for Checkbox<T> {
impl Widget for Checkbox {
fn on_pointer_event(&mut self, ctx: &mut EventCtx, event: &PointerEvent) {
match event {
PointerEvent::PointerDown(_, _) => {
Expand Down Expand Up @@ -280,9 +282,9 @@ mod tests {
let mut harness = TestHarness::create_with_size(checkbox, Size::new(50.0, 50.0));

harness.edit_root_widget(|mut checkbox| {
let mut checkbox = checkbox.downcast::<Checkbox<&'static str>>();
let mut checkbox = checkbox.downcast::<Checkbox>();
checkbox.set_checked(true);
checkbox.set_text("The quick brown fox jumps over the lazy dog");
checkbox.set_text(ArcStr::from("The quick brown fox jumps over the lazy dog"));

let mut label = checkbox.label_mut();
label.set_text_brush(PRIMARY_LIGHT);
Expand Down
2 changes: 1 addition & 1 deletion crates/masonry/src/widget/flex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1392,7 +1392,7 @@ mod tests {
let mut child = flex.child_mut(1).unwrap();
assert_eq!(
child
.try_downcast::<Label<&'static str>>()
.try_downcast::<Label>()
.unwrap()
.widget
.text()
Expand Down
34 changes: 18 additions & 16 deletions crates/masonry/src/widget/label.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,26 +34,29 @@ pub enum LineBreaking {
}

/// A widget displaying non-editable text.
pub struct Label<T: TextStorage> {
text_layout: TextLayout<T>,
pub struct Label {
// We hardcode the underlying storage type as `ArcStr` for `Label`
// More advanced use cases will almost certainly need a custom widget, anyway
// (Rich text is not yet fully integrated, and so the architecture by which a label
// has rich text properties specified still needs to be designed)
text_layout: TextLayout<ArcStr>,
line_break_mode: LineBreaking,
show_disabled: bool,
brush: TextBrush,
// disabled: bool,
}

impl<T: TextStorage> Label<T> {
impl Label {
/// Create a new label.
pub fn new(text: T) -> Self {
pub fn new(text: impl Into<ArcStr>) -> Self {
Self {
text_layout: TextLayout::new(text, crate::theme::TEXT_SIZE_NORMAL as f32),
text_layout: TextLayout::new(text.into(), crate::theme::TEXT_SIZE_NORMAL as f32),
line_break_mode: LineBreaking::Overflow,
show_disabled: true,
brush: crate::theme::TEXT_COLOR.into(),
}
}

pub fn text(&self) -> &T {
pub fn text(&self) -> &ArcStr {
self.text_layout.text()
}

Expand Down Expand Up @@ -85,29 +88,28 @@ impl<T: TextStorage> Label<T> {
self.line_break_mode = line_break_mode;
self
}
}

impl Label<ArcStr> {
/// Create a label with empty text.
pub fn empty() -> Self {
Self::new("".into())
Self::new("")
}
}

impl<T: TextStorage> WidgetMut<'_, Label<T>> {
pub fn text(&self) -> &T {
impl WidgetMut<'_, Label> {
pub fn text(&self) -> &ArcStr {
self.widget.text_layout.text()
}

pub fn set_text_properties<R>(&mut self, f: impl FnOnce(&mut TextLayout<T>) -> R) -> R {
pub fn set_text_properties<R>(&mut self, f: impl FnOnce(&mut TextLayout<ArcStr>) -> R) -> R {
let ret = f(&mut self.widget.text_layout);
if self.widget.text_layout.needs_rebuild() {
self.ctx.request_layout();
}
ret
}

pub fn set_text(&mut self, new_text: T) {
pub fn set_text(&mut self, new_text: impl Into<ArcStr>) {
let new_text = new_text.into();
self.set_text_properties(|layout| layout.set_text(new_text));
}

Expand Down Expand Up @@ -138,7 +140,7 @@ impl<T: TextStorage> WidgetMut<'_, Label<T>> {
}
}

impl<T: TextStorage> Widget for Label<T> {
impl Widget for Label {
fn on_pointer_event(&mut self, _ctx: &mut EventCtx, event: &PointerEvent) {
match event {
PointerEvent::PointerMove(_point) => {
Expand Down Expand Up @@ -353,7 +355,7 @@ mod tests {
let mut harness = TestHarness::create_with_size(label, Size::new(50.0, 50.0));

harness.edit_root_widget(|mut label| {
let mut label = label.downcast::<Label<&'static str>>();
let mut label = label.downcast::<Label>();
label.set_text("The quick brown fox jumps over the lazy dog");
label.set_text_brush(PRIMARY_LIGHT);
label.set_font_family(FontFamily::Generic(GenericFamily::Monospace));
Expand Down
41 changes: 27 additions & 14 deletions crates/masonry/src/widget/prose.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@ use tracing::trace;
use vello::{peniko::BlendMode, Scene};

use crate::{
text2::{Selectable, TextBrush, TextWithSelection},
text2::{TextBrush, TextStorage, TextWithSelection},
widget::label::LABEL_X_PADDING,
AccessCtx, AccessEvent, BoxConstraints, EventCtx, LayoutCtx, LifeCycle, LifeCycleCtx, PaintCtx,
PointerEvent, StatusChange, TextEvent, Widget,
AccessCtx, AccessEvent, ArcStr, BoxConstraints, EventCtx, LayoutCtx, LifeCycle, LifeCycleCtx,
PaintCtx, PointerEvent, StatusChange, TextEvent, Widget,
};

use super::{LineBreaking, WidgetMut, WidgetRef};
Expand All @@ -26,25 +26,26 @@ use super::{LineBreaking, WidgetMut, WidgetRef};
///
/// This should be preferred over [`Label`](super::Label) for most
/// immutable text, other than that within
pub struct Prose<T: Selectable> {
text_layout: TextWithSelection<T>,
pub struct Prose {
// See `Label` for discussion of the choice of text type
text_layout: TextWithSelection<ArcStr>,
line_break_mode: LineBreaking,
show_disabled: bool,
brush: TextBrush,
}

impl<T: Selectable> Prose<T> {
pub fn new(text: T) -> Self {
impl Prose {
pub fn new(text: impl Into<ArcStr>) -> Self {
Prose {
text_layout: TextWithSelection::new(text, crate::theme::TEXT_SIZE_NORMAL as f32),
text_layout: TextWithSelection::new(text.into(), crate::theme::TEXT_SIZE_NORMAL as f32),
line_break_mode: LineBreaking::WordWrap,
show_disabled: true,
brush: crate::theme::TEXT_COLOR.into(),
}
}

// TODO: Can we reduce code duplication with `Label` widget somehow?
pub fn text(&self) -> &T {
pub fn text(&self) -> &ArcStr {
self.text_layout.text()
}

Expand Down Expand Up @@ -79,20 +80,31 @@ impl<T: Selectable> Prose<T> {
}
}

impl<T: Selectable> WidgetMut<'_, Prose<T>> {
pub fn text(&self) -> &T {
impl WidgetMut<'_, Prose> {
pub fn text(&self) -> &ArcStr {
self.widget.text_layout.text()
}

pub fn set_text_properties<R>(&mut self, f: impl FnOnce(&mut TextWithSelection<T>) -> R) -> R {
pub fn set_text_properties<R>(
&mut self,
f: impl FnOnce(&mut TextWithSelection<ArcStr>) -> R,
) -> R {
let ret = f(&mut self.widget.text_layout);
if self.widget.text_layout.needs_rebuild() {
self.ctx.request_layout();
}
ret
}

pub fn set_text(&mut self, new_text: T) {
/// Change the text. If the user currently has a selection in the box, this will delete that selection.
///
/// We enforce this to be an `ArcStr` to make the allocation explicit.
pub fn set_text(&mut self, new_text: ArcStr) {
if self.ctx.is_focused() {
tracing::info!(
"Called reset_text on a focused `Prose`. This will lose the user's current selection"
);
}
self.set_text_properties(|layout| layout.set_text(new_text));
}

Expand Down Expand Up @@ -123,7 +135,7 @@ impl<T: Selectable> WidgetMut<'_, Prose<T>> {
}
}

impl<T: Selectable> Widget for Prose<T> {
impl Widget for Prose {
fn on_pointer_event(&mut self, ctx: &mut EventCtx, event: &PointerEvent) {
let window_origin = ctx.widget_state.window_origin();
let inner_origin = Point::new(window_origin.x + LABEL_X_PADDING, window_origin.y);
Expand Down Expand Up @@ -210,6 +222,7 @@ impl<T: Selectable> Widget for Prose<T> {
ctx.request_layout();
}
LifeCycle::BuildFocusChain => {
// TODO: This is *definitely* empty
if !self.text_layout.text().links().is_empty() {
tracing::warn!("Links present in text, but not yet integrated");
}
Expand Down
Loading

0 comments on commit 9542aaf

Please sign in to comment.