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

Message posting clean-up #4256

Merged
merged 30 commits into from
Mar 7, 2024
Merged
Show file tree
Hide file tree
Changes from 29 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
f0dfa4c
Tidy up a couple of typing issues in the Collapsible tests
davep Mar 4, 2024
49b03d5
Correct a Collapsible's test description
davep Mar 4, 2024
79341a5
Add a test for getting a message when changing collapsible via reactive
davep Mar 4, 2024
6b96f20
Correct a Collapsible's test description
davep Mar 4, 2024
f45df6a
Add a test for getting a message when changing collapsible via reactive
davep Mar 4, 2024
25e3780
Ensure Collapsed/Expanded message get sent on reactive change
davep Mar 4, 2024
161576a
Add a test for Markdown.TableOfContentsUpdated being posted
davep Mar 4, 2024
8fd51d2
Ensure that TableOfContentsUpdated is always posted
davep Mar 4, 2024
d284ada
Add a test for getting Select.Changed on value change
davep Mar 4, 2024
f8dfe82
Ensure Select.Changed is posted when value is changed
davep Mar 4, 2024
4a3fbe6
Add a test that SelectionList.toggle results in SelectionToggled
davep Mar 4, 2024
01f09da
Ensure SelectionList.SelectionChanged gets posted in all DOM shapes
davep Mar 4, 2024
09bc439
Ensure SelectionList.SelectionToggled is posted when calling toggle
davep Mar 4, 2024
40a40d8
Update snapshot tests
davep Mar 4, 2024
1872057
Fix the sender for Cleared when calling clear_panes
davep Mar 5, 2024
6b30ff2
Fix the sender for Cleared when calling remove_pane
davep Mar 5, 2024
e245562
Ensure correct sender for Tabs.TabHidden in all DOM shapes
davep Mar 5, 2024
51aa01e
Ensure correct sender for Tabs.TabShown in all DOM shapes
davep Mar 5, 2024
5bb61a7
Tidy up trailing whitespace
davep Mar 5, 2024
5acdee9
Post TextArea.Changed when TextArea.text is assigned
davep Mar 5, 2024
8e630d7
Ensure correct sender when posting TextArea.Changed
davep Mar 5, 2024
99928bc
Move posting TextArea.Changed to TextArea.load_text
davep Mar 5, 2024
bc74e1d
Ensure Tree.NodeExpanded is bubbbled in all DOM shapes
davep Mar 5, 2024
f8303af
Allow different expanded states in the extended tree tester
davep Mar 5, 2024
d95c3e9
Merge branch 'main' into message-tidy
davep Mar 5, 2024
b73f9c5
Ensure Tree.NodeCollapsed is bubbbled in all DOM shapes
davep Mar 5, 2024
f6651ec
Remove a message we're not testing
davep Mar 5, 2024
ec2677c
Merge branch 'main' into message-tidy
davep Mar 6, 2024
a4d123b
Mention the message changes in the ChangeLog
davep Mar 6, 2024
0d4de0b
Swap to using update
davep Mar 7, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,15 @@ and this project adheres to [Semantic Versioning](http://semver.org/).

- Clicking a non focusable widget focus ancestors https://github.com/Textualize/textual/pull/4236
- BREAKING: widget class names must start with a capital letter or an underscore `_` https://github.com/Textualize/textual/pull/4252
- BREAKING: for many widgets, messages are now sent when programmatic changes that mirror user input are made https://github.com/Textualize/textual/pull/4256
- Changed `Collapsible`
- Changed `Markdown`
- Changed `Select`
- Changed `SelectionList`
- Changed `TabbedContent`
- Changed `Tabs`
- Changed `TextArea`
- Changed `Tree`

## [0.52.1] - 2024-02-20

Expand Down
10 changes: 5 additions & 5 deletions src/textual/widgets/_collapsible.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ def _watch_collapsed(self, collapsed: bool) -> None:
class Collapsible(Widget):
"""A collapsible container."""

collapsed = reactive(True)
collapsed = reactive(True, init=False)
title = reactive("Toggle")

DEFAULT_CSS = """
Expand Down Expand Up @@ -194,14 +194,14 @@ def __init__(
def _on_collapsible_title_toggle(self, event: CollapsibleTitle.Toggle) -> None:
event.stop()
self.collapsed = not self.collapsed
if self.collapsed:
self.post_message(self.Collapsed(self))
else:
self.post_message(self.Expanded(self))

def _watch_collapsed(self, collapsed: bool) -> None:
"""Update collapsed state when reactive is changed."""
self._update_collapsed(collapsed)
if self.collapsed:
self.post_message(self.Collapsed(self))
else:
self.post_message(self.Expanded(self))

def _update_collapsed(self, collapsed: bool) -> None:
"""Update children to match collapsed state."""
Expand Down
4 changes: 3 additions & 1 deletion src/textual/widgets/_markdown.py
Original file line number Diff line number Diff line change
Expand Up @@ -934,7 +934,9 @@ def update(self, markdown: str) -> AwaitComplete:
(stack[-1]._blocks if stack else output).append(external)

self.post_message(
Markdown.TableOfContentsUpdated(self, self._table_of_contents)
Markdown.TableOfContentsUpdated(self, self._table_of_contents).set_sender(
self
)
)
markdown_block = self.query("MarkdownBlock")

Expand Down
6 changes: 4 additions & 2 deletions src/textual/widgets/_select.py
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,9 @@ class Select(Generic[SelectType], Vertical, can_focus=True):
"""True to show the overlay, otherwise False."""
prompt: var[str] = var[str]("Select")
"""The prompt to show when no value is selected."""
value: var[SelectType | NoSelection] = var[Union[SelectType, NoSelection]](BLANK)
value: var[SelectType | NoSelection] = var[Union[SelectType, NoSelection]](
BLANK, init=False
)
"""The value of the selection.

If the widget has no selection, its value will be [`Select.BLANK`][textual.widgets.Select.BLANK].
Expand Down Expand Up @@ -459,6 +461,7 @@ def _watch_value(self, value: SelectType | NoSelection) -> None:
select_overlay.highlighted = index
select_current.update(prompt)
break
self.post_message(self.Changed(self, value))

def compose(self) -> ComposeResult:
"""Compose Select with overlay and current value."""
Expand Down Expand Up @@ -509,7 +512,6 @@ def _update_selection(self, event: SelectOverlay.UpdateSelection) -> None:
value = self._options[event.option_index][1]
if value != self.value:
self.value = value
self.post_message(self.Changed(self, value))

async def update_focus() -> None:
"""Update focus and reset overlay."""
Expand Down
45 changes: 38 additions & 7 deletions src/textual/widgets/_selection_list.py
Original file line number Diff line number Diff line change
Expand Up @@ -252,14 +252,19 @@ def __init__(
"""Tracking of which values are selected."""
self._send_messages = False
"""Keep track of when we're ready to start sending messages."""
options = [self._make_selection(selection) for selection in selections]
super().__init__(
*[self._make_selection(selection) for selection in selections],
*options,
name=name,
id=id,
classes=classes,
disabled=disabled,
wrap=False,
)
self._values: dict[SelectionType, int] = {
option.value: index for index, option in enumerate(options)
}
"""Keeps track of which value relates to which option."""

@property
def selected(self) -> list[SelectionType]:
Expand All @@ -285,7 +290,20 @@ def _message_changed(self) -> None:
messages.
"""
if self._send_messages:
self.post_message(self.SelectedChanged(self))
self.post_message(self.SelectedChanged(self).set_sender(self))

def _message_toggled(self, option_index: int) -> None:
"""Post a message that an option was toggled, where appropriate.

Note:
A message will only be sent if `_send_messages` is `True`. This
makes this safe to call before the widget is ready for posting
messages.
"""
if self._send_messages:
self.post_message(
self.SelectionToggled(self, option_index).set_sender(self)
)

def _apply_to_all(self, state_change: Callable[[SelectionType], bool]) -> Self:
"""Apply a selection state change to all selection options in the list.
Expand All @@ -306,9 +324,9 @@ def _apply_to_all(self, state_change: Callable[[SelectionType], bool]) -> Self:
changed = False

# Next we run through everything and apply the change, preventing
# the changed message because the caller really isn't going to be
# expecting a message storm from this.
with self.prevent(self.SelectedChanged):
# the toggled and changed messages because the caller really isn't
# going to be expecting a message storm from this.
with self.prevent(self.SelectedChanged, self.SelectionToggled):
for selection in self._options:
changed = (
state_change(cast(Selection[SelectionType], selection).value)
Expand Down Expand Up @@ -416,6 +434,7 @@ def _toggle(self, value: SelectionType) -> bool:
self._deselect(value)
else:
self._select(value)
self._message_toggled(self._values[value])
return True

def toggle(self, selection: Selection[SelectionType] | SelectionType) -> Self:
Expand Down Expand Up @@ -593,7 +612,6 @@ def _on_option_list_option_selected(self, event: OptionList.OptionSelected) -> N
"""
event.stop()
self._toggle_highlighted_selection()
self.post_message(self.SelectionToggled(self, event.option_index))

def get_option_at_index(self, index: int) -> Selection[SelectionType]:
"""Get the selection option at the given index.
Expand Down Expand Up @@ -632,7 +650,9 @@ def _remove_option(self, index: int) -> None:
Raises:
IndexError: If there is no selection option of the given index.
"""
self._deselect(self.get_option_at_index(index).value)
option = self.get_option_at_index(index)
self._deselect(option.value)
del self._values[option.value]
return super()._remove_option(index)

def add_options(
Expand Down Expand Up @@ -679,6 +699,16 @@ def add_options(
raise SelectionError(
"Only Selection or a prompt/value tuple is supported in SelectionList"
)

# Add the new items to the value mappings.
self._values = {
**self._values,
**{
option.value: index
for index, option in enumerate(cleaned_options, start=self.option_count)
},
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't .update better in terms of speed while being the same in terms of readability?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Off the top of my head I don't know the speed difference; it feels like you are right, do you have a reference?

Copy link
Contributor

@rodrigogiraoserrao rodrigogiraoserrao Mar 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have a reference, no, it's just an educated guess.
I was just surprised you spelled this out 🤷

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd probably go with update. It would be faster, but doubt the difference would be measurable. It's more that update expresses intent better.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool. Will tweak it.


return super().add_options(cleaned_options)

def add_option(
Expand Down Expand Up @@ -711,4 +741,5 @@ def clear_options(self) -> Self:
The `SelectionList` instance.
"""
self._selected.clear()
self._values.clear()
return super().clear_options()
20 changes: 6 additions & 14 deletions src/textual/widgets/_tabbed_content.py
Original file line number Diff line number Diff line change
Expand Up @@ -466,16 +466,12 @@ def remove_pane(self, pane_id: str) -> AwaitComplete:
# other means; so allow that to be a no-op.
pass

async def _remove_content(cleared_message: TabbedContent.Cleared) -> None:
async def _remove_content() -> None:
await gather(*removal_awaitables)
if self.tab_count == 0:
self.post_message(cleared_message)
self.post_message(self.Cleared(self).set_sender(self))

# Note that I create the Cleared message out here, rather than in
# _remove_content, to ensure that the message's internal
# understanding of who the sender is is correct.
# https://github.com/Textualize/textual/issues/2750
return AwaitComplete(_remove_content(self.Cleared(self)))
return AwaitComplete(_remove_content())

def clear_panes(self) -> AwaitComplete:
"""Remove all the panes in the tabbed content.
Expand All @@ -489,15 +485,11 @@ def clear_panes(self) -> AwaitComplete:
self.get_child_by_type(ContentSwitcher).remove_children(),
)

async def _clear_content(cleared_message: TabbedContent.Cleared) -> None:
async def _clear_content() -> None:
await await_clear
self.post_message(cleared_message)
self.post_message(self.Cleared(self).set_sender(self))

# Note that I create the Cleared message out here, rather than in
# _clear_content, to ensure that the message's internal
# understanding of who the sender is is correct.
# https://github.com/Textualize/textual/issues/2750
return AwaitComplete(_clear_content(self.Cleared(self)))
return AwaitComplete(_clear_content())

def compose_add_child(self, widget: Widget) -> None:
"""When using the context manager compose syntax, we want to attach nodes to the switcher.
Expand Down
4 changes: 2 additions & 2 deletions src/textual/widgets/_tabs.py
Original file line number Diff line number Diff line change
Expand Up @@ -797,7 +797,7 @@ def hide(self, tab_id: str) -> Tab:
next_tab = self._next_active
self.active = next_tab.id or "" if next_tab else ""
tab_to_hide.add_class("-hidden")
self.post_message(self.TabHidden(self, tab_to_hide))
self.post_message(self.TabHidden(self, tab_to_hide).set_sender(self))
self.call_after_refresh(self._highlight_active)
return tab_to_hide

Expand All @@ -820,7 +820,7 @@ def show(self, tab_id: str) -> Tab:
raise self.TabError(f"There is no tab with ID {tab_id!r} to show.")

tab_to_show.remove_class("-hidden")
self.post_message(self.TabShown(self, tab_to_show))
self.post_message(self.TabShown(self, tab_to_show).set_sender(self))
if not self.active:
self._activate_tab(tab_to_show)
self.call_after_refresh(self._highlight_active)
Expand Down
27 changes: 14 additions & 13 deletions src/textual/widgets/_text_area.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,33 +92,33 @@ class TextArea(ScrollView):
height: 1fr;
border: tall $background;
padding: 0 1;

& .text-area--gutter {
color: $text 40%;
}

& .text-area--cursor-gutter {
color: $text 60%;
background: $boost;
text-style: bold;
}

& .text-area--cursor-line {
background: $boost;
}

& .text-area--selection {
background: $accent-lighten-1 40%;
}

& .text-area--matching-bracket {
background: $foreground 30%;
}

&:focus {
border: tall $accent;
}

&:dark {
.text-area--cursor {
color: $text 90%;
Expand All @@ -128,11 +128,11 @@ class TextArea(ScrollView):
background: $warning-darken-1;
}
}

&:light {
.text-area--cursor {
color: $text 90%;
background: $foreground 70%;
background: $foreground 70%;
}
&.-read-only .text-area--cursor {
background: $warning-darken-1;
Expand All @@ -151,9 +151,9 @@ class TextArea(ScrollView):
}
"""
`TextArea` offers some component classes which can be used to style aspects of the widget.

Note that any attributes provided in the chosen `TextAreaTheme` will take priority here.

| Class | Description |
| :- | :- |
| `text-area--cursor` | Target the cursor. |
Expand Down Expand Up @@ -313,9 +313,9 @@ class TextArea(ScrollView):

read_only: Reactive[bool] = reactive(False)
"""True if the content is read-only.

Read-only means end users cannot insert, delete or replace content.

The document can still be edited programmatically via the API.
"""

Expand Down Expand Up @@ -883,6 +883,7 @@ def load_text(self, text: str) -> None:
"""
self.history.clear()
self._set_document(text, self.language)
self.post_message(self.Changed(self).set_sender(self))

def _on_resize(self) -> None:
self._rewrap_and_refresh_virtual_size()
Expand Down
4 changes: 2 additions & 2 deletions src/textual/widgets/_tree.py
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ def _expand(self, expand_all: bool) -> None:
"""
self._expanded = True
self._updates += 1
self._tree.post_message(Tree.NodeExpanded(self))
self._tree.post_message(Tree.NodeExpanded(self).set_sender(self._tree))
if expand_all:
for child in self.children:
child._expand(expand_all)
Expand Down Expand Up @@ -248,7 +248,7 @@ def _collapse(self, collapse_all: bool) -> None:
"""
self._expanded = False
self._updates += 1
self._tree.post_message(Tree.NodeCollapsed(self))
self._tree.post_message(Tree.NodeCollapsed(self).set_sender(self._tree))
if collapse_all:
for child in self.children:
child._collapse(collapse_all)
Expand Down
10 changes: 10 additions & 0 deletions tests/select/test_changed_message.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,3 +55,13 @@ async def test_same_selection_does_not_post_message():
await pilot.click(SelectOverlay, offset=(2, 3))
await pilot.pause()
assert len(app.changed_messages) == 1


async def test_setting_value_posts_message() -> None:
"""Setting the value of a Select should post a message."""

async with (app := SelectApp()).run_test() as pilot:
assert len(app.changed_messages) == 0
app.query_one(Select).value = 2
await pilot.pause()
assert len(app.changed_messages) == 1
17 changes: 11 additions & 6 deletions tests/selection_list/test_selection_messages.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,17 +28,21 @@ def compose(self) -> ComposeResult:
@on(SelectionList.SelectedChanged)
def _record(
self,
event: OptionList.OptionMessage
| SelectionList.SelectionMessage
| SelectionList.SelectedChanged,
event: (
OptionList.OptionMessage
| SelectionList.SelectionMessage
| SelectionList.SelectedChanged
),
) -> None:
assert event.control == self.query_one(SelectionList)
self.messages.append(
(
event.__class__.__name__,
event.selection_index
if isinstance(event, SelectionList.SelectionMessage)
else None,
(
event.selection_index
if isinstance(event, SelectionList.SelectionMessage)
else None
),
)
)

Expand Down Expand Up @@ -71,6 +75,7 @@ async def test_toggle() -> None:
assert pilot.app.messages == [
("SelectionHighlighted", 0),
("SelectedChanged", None),
("SelectionToggled", 0),
]


Expand Down
Loading
Loading