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

Added select_all method to TextInput. #776

Merged
merged 3 commits into from
Jul 22, 2021
Merged

Added select_all method to TextInput. #776

merged 3 commits into from
Jul 22, 2021

Conversation

AldoMX
Copy link
Contributor

@AldoMX AldoMX commented Mar 13, 2021

Hi, and thank you for creating iced.

I was tinkering with the todos example and I wanted to pre-select the text when you clicked the edit button.

But when I tried to update the example I noticed that the public methods from TextInput only allowed to move the cursor, so I added a select_all method and updated the todos example to achieve the desired outcome.

@hecrj hecrj added the feature New feature or request label Jul 22, 2021
@hecrj hecrj added this to the 0.4.0 milestone Jul 22, 2021
... just for readability
Copy link
Member

@hecrj hecrj left a comment

Choose a reason for hiding this comment

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

Works great! Thank you 🎉

I have added the method to iced_web too, although we can't properly implement it yet! This way, the todos example can still be built for web.

Copy link
Member

@derezzedex derezzedex left a comment

Choose a reason for hiding this comment

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

I've left a comment regarding the proposed function. There's also the need to implement this on the web backend, but I'm not sure how that works because I'm not familiar enough with it.

Comment on lines +797 to +799
pub fn select_all(&mut self) {
self.cursor.select_range(0, usize::MAX);
}
Copy link
Member

Choose a reason for hiding this comment

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

I think it would make more sense to expose a more general select function, accepting both the start and the end, then in case you needed to select all you would use it just like you currently call cursor.select_range.

I'm not sure if there's a way to clip the range to the actual text size, since the Value is not available for the State, but right now this is done anyway on the Renderer: https://github.com/hecrj/iced/blob/f076649fbb575ee036ab0b3f4511690e3379c115/graphics/src/widget/text_input.rs#L157-L158

Copy link
Member

Choose a reason for hiding this comment

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

I think select_all satisfies a clear use case.

We can expose a select_range or similar and focus on those use cases in a different unit of work.

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

Successfully merging this pull request may close these issues.

4 participants