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

Messages shouldn't be suppressed in programmatic changes #2500

Closed
rodrigogiraoserrao opened this issue May 5, 2023 · 2 comments
Closed

Messages shouldn't be suppressed in programmatic changes #2500

rodrigogiraoserrao opened this issue May 5, 2023 · 2 comments
Labels
bug Something isn't working question Further information is requested

Comments

@rodrigogiraoserrao
Copy link
Contributor

rodrigogiraoserrao commented May 5, 2023

TL;DR: We should not prevent a message from being posted just because the state of the widget was changed programmatically, and even though the programmer already knows the state changed.

Immediate effect: TabbedContent will now post TabActivated when the reactive attribute active is assigned to.

Possible further considerations:

  • Add a message Completed to the ProgressBar widget.
  • Add a message Started to the ProgressBar widget.
  • ...?

Thesis:
Most widgets will post appropriate messages when their state changes, even if through programmatic means.
E.g.,

  • DataTable will post CellHighlighted if we move the cursor with .move_cursor or by assigning directly to cursor_coordinate;
  • Checkbox and RadioButton will post Changed if we toggle them with .toggle or by assigning directly to value; and
  • OptionList will post OptionHighlighted if we assign to the reactive highlighted.

I think all programmatic changes should still post the appropriate messages that reflect the changes that happened.

Even if a programmer does changes like this programmatically, they are sensible in assuming that the respective events will be triggered.
This lets them decouple two things:

  • the logic that will change some state; and
  • the logic that handles changes to the state.

If a programmer does not want the message, they can explicitly opt-out with the context manager prevents, which is why the context manager was created in the first place.

To consider if this is accepted.
  1. For the issue with TabbedContent: the usage of prevent in the TabbedContent comes from Prevent reactive-watcher loop in Tabs / TabbedContent. #2305 and Revert "Prevent reactive-watcher loop in Tabs / TabbedContent." #2322, so take those two PRs into account when fixing this.
  2. Consider if there are other events missing that a user might want to be notified about.
@rodrigogiraoserrao rodrigogiraoserrao added bug Something isn't working question Further information is requested labels May 5, 2023
@rodrigogiraoserrao
Copy link
Contributor Author

Settled with #3417.

We post all messages.

Copy link

Don't forget to star the repository!

Follow @textualizeio for Textual updates.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working question Further information is requested
Projects
None yet
Development

No branches or pull requests

1 participant