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

Add a theme constant to change LineEdit and TextEdit's caret width #54956

Conversation

Calinou
Copy link
Member

@Calinou Calinou commented Nov 13, 2021

This can be useful to improve caret visibility, especially at larger font sizes. This can also be used for accessibility purposes.

This closes godotengine/godot-proposals#1220.

Preview

2021-11-13_23 06 08

2021-11-13_23 06 13

@Calinou Calinou requested review from a team as code owners November 13, 2021 22:11
@Calinou Calinou added this to the 4.0 milestone Nov 13, 2021
@YuriSizov
Copy link
Contributor

Shouldn't we always multiply it by scale? And thus the default would be 1.

@Paulb23
Copy link
Member

Paulb23 commented Nov 20, 2021

This doesn't seem to close godotengine/godot-proposals#1220?

Not sure about exposing the width, but rather we should port the TextEdit::CaretType over add more options.

@Calinou
Copy link
Member Author

Calinou commented Nov 20, 2021

This doesn't seem to close godotengine/godot-proposals#1220?

This PR provides a close enough solution for the 90% use case. Using arbitrary textures for the caret is a very uncommon use case, and I don't feel it's worth supporting in core. As long as there's a way to get the caret position in pixels, it's possible to hide the default caret by setting its color to Color(0, 0, 0, 0) and draw a textured caret manually.

Copy link
Member

@bruvzg bruvzg left a comment

Choose a reason for hiding this comment

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

Shouldn't we always multiply it by scale? And thus the default would be 1.

I agree, it's probably better to always use theme scaling, otherwise looks good.

scene/gui/line_edit.cpp Outdated Show resolved Hide resolved
@Calinou Calinou force-pushed the lineedit-textedit-add-caret-width-theme-item branch from ea51fe1 to 18f64d6 Compare November 22, 2021 15:15
@Calinou Calinou force-pushed the lineedit-textedit-add-caret-width-theme-item branch from 18f64d6 to ed175e1 Compare January 7, 2022 18:39
@Calinou
Copy link
Member Author

Calinou commented Jan 7, 2022

Rebased and tested again, it works as expected.

@YuriSizov
Copy link
Contributor

Shouldn't we always multiply it by scale? And thus the default would be 1.

Will you address this concern? 🙃

@Calinou Calinou force-pushed the lineedit-textedit-add-caret-width-theme-item branch from ed175e1 to 32c6bc2 Compare January 7, 2022 19:49
@Calinou
Copy link
Member Author

Calinou commented Jan 7, 2022

Will you address this concern? 🙃

Done (and tested again) 🙂

I thought I had addressed it before, but I didn't.

@Calinou Calinou force-pushed the lineedit-textedit-add-caret-width-theme-item branch 2 times, most recently from 91bac8a to 696102c Compare January 7, 2022 19:51
This can be useful to improve caret visibility, especially at
larger font sizes. This can also be used for accessibility purposes.
@Calinou Calinou force-pushed the lineedit-textedit-add-caret-width-theme-item branch from 696102c to f6443be Compare January 7, 2022 19:51
@YuriSizov YuriSizov requested review from a team and bruvzg January 8, 2022 15:45
@akien-mga akien-mga merged commit f6792ea into godotengine:master Jan 13, 2022
@akien-mga
Copy link
Member

Thanks!

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

Successfully merging this pull request may close these issues.

Allow to customize caret symbol in LineEdit
5 participants