Skip to content

chore: add documentation to radio_button.rs and fix warning in image.rs #861

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

AncutaIoan
Copy link

No description provided.

@AncutaIoan
Copy link
Author

616

Copy link
Collaborator

@jrmoulton jrmoulton left a comment

Choose a reason for hiding this comment

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

Thanks for helping to improve the docs! There are some good changes in here but in some other places I feel like your changes are removing clarity. Can you explain why you made the changes?

Comment on lines -36 to +79
/// Creates a new radio button with a closure that determines its selected state.
/// Creates a new radio button using a closure for its current value.
///
/// The returned value is wrapped in a [`ValueContainer`] so it can be managed declaratively.
///
/// This method is useful when you want a radio button whose state is determined by a closure.
/// The state can be dynamically updated by the closure, and the radio button will reflect these changes.
#[allow(clippy::new_ret_no_self)]
/// # Example
/// ```rust
/// use floem::views::RadioButton;
/// use floem_reactive::{RwSignal, SignalGet};
/// let selected = RwSignal::new("A".to_string());
/// RadioButton::new("A".to_string(), move || selected.get());
/// ```
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure that this is an improvement over the previous docs. Why did you change it?

Comment on lines -59 to +98
/// Use this method when you have a signal that provides the current state of the radio button.
/// The radio button will automatically update its state based on the signal.
/// Useful for when the button state is externally managed and shouldn't be changed by the user.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think this is an improvement over the previous docs.

Comment on lines -73 to +111
/// This method is ideal when you need a radio button that not only reflects a signal's state but also updates it.
/// Clicking the radio button will set the signal to the represented value.
/// When selected, the radio button will set the underlying signal to its represented value.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why this change?

Comment on lines -91 to +136
/// Creates a new labeled radio button with a closure that determines its selected state.
///
/// This method is useful when you want a labeled radio button whose state is determined by a closure.
/// The label is also provided by a closure, allowing for dynamic updates.
/// Creates a new **labeled** radio button from a closure and label generator.
Copy link
Collaborator

Choose a reason for hiding this comment

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

same thing here. I think this is removing clarity.

Comment on lines -120 to +162
/// Creates a new labeled radio button with a signal that provides its selected state.
///
/// Use this method when you have a signal that provides the current state of the radio button and you also want a label.
/// The radio button and label will automatically update based on the signal.
/// Creates a read-only **labeled** radio button from a signal and label.
Copy link
Collaborator

Choose a reason for hiding this comment

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

same thing

Comment on lines -141 to +180
/// Creates a new labeled radio button with a signal that provides and updates its selected state.
///
/// This method is ideal when you need a labeled radio button that not only reflects a signal's state but also updates it.
/// Clicking the radio button will set the signal to the represented value.
/// Creates a reactive **labeled** radio button with two-way binding and dynamic label.
Copy link
Collaborator

Choose a reason for hiding this comment

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

same here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants