Skip to content

Conversation

StephanvanSchaik
Copy link

@StephanvanSchaik StephanvanSchaik commented Jul 24, 2025

I am currently using a text input field in xilem as a password field, but there is currently no way to hide the text by replacing the actual buffer of characters with a repeated character such as an asterisk (i.e., '*'). Further looking into xilem and masonry, I found that the text input field leverages PlainEditor in parley, so I am trying to extend that first.

More specifically, this commit adds a visible_buffer field to hold the character buffer with the replaced characters and a hide_symbol field to hold the character that needs to be shown (if any is set). This is to ensure that the original buffer field stays intact everywhere else (together with the corresponding logic), and this seems to be the least intrusive way to achieve this. Further, I have extended update_layout to update the visible_buffer field if the symbol or the length of the original buffer changed and to decide which buffer needs to be shown based on whether the symbol is set or not. Finally, this adds two methods to set and clear the symbol used for hiding the text.

Extending this functionality to masonry/xilem should be as simple as calling those two methods once this lands. I think this is also neat because this makes it simple to add a show/hide password checkbox if desired.

Issue #327 is related, but we should keep that open even if this lands, because there are probably a bunch of corner cases or additional desired features, I am just trying to get the most awkward part out of the way which is that the password shouldn't be visible on the screen.

@StephanvanSchaik
Copy link
Author

StephanvanSchaik commented Jul 24, 2025

I see that cargo clippy fails, but this seems to be affecting source files that this PR does not touch. I can fix the issues if desired.

EDIT: I figured out how to expand the command output in the Github CI and fixed the no_std issues. clippy still seems to complain locally, which is what led to the confusion.

@nicoburns
Copy link
Contributor

I'm also very keen to have this functionality, but I think we'll need more sophisticated logic to handle character counting (one char does not always correspond to one visible character), selection and editing.

@StephanvanSchaik
Copy link
Author

StephanvanSchaik commented Jul 25, 2025

I'm also very keen to have this functionality, but I think we'll need more sophisticated logic to handle character counting (one char does not always correspond to one visible character), selection and editing.

Oh, that is right, I forgot about this entirely (it is sometimes too easy to forget). Thanks for the reminder.

Would it be fine to use the unicode-segmentation crate to count the grapheme clusters? It is currently not a dependency and I am not sure how parley intends on doing Unicode segmentation (i.e., what dependency to use for that functionality).

@xorgy
Copy link
Member

xorgy commented Jul 27, 2025

This is a desirable feature, but this is not the right design I think. Introducing more segmentation is not necessary if we use a style instead, for example; and it lets you set it up simply with a default style, or a range/tree style (and eventually, through Attributed Text).
Adding extra state to the editor, separate segmentation rules, etc. is not how we would solve this.

My suggestion is to address it through styles, call it grapheme replacement or cluster substitution or something, maybe make it use a &'static str (or a fixed length string type which reasonably accommodates values)... but there may be complications with that, which I have not thought of.

@StephanvanSchaik StephanvanSchaik force-pushed the passwords branch 2 times, most recently from 10bc321 to 755da0b Compare August 4, 2025 00:10
@StephanvanSchaik
Copy link
Author

This is a desirable feature, but this is not the right design I think. Introducing more segmentation is not necessary if we use a style instead, for example; and it lets you set it up simply with a default style, or a range/tree style (and eventually, through Attributed Text). Adding extra state to the editor, separate segmentation rules, etc. is not how we would solve this.

My suggestion is to address it through styles, call it grapheme replacement or cluster substitution or something, maybe make it use a &'static str (or a fixed length string type which reasonably accommodates values)... but there may be complications with that, which I have not thought of.

Thanks for the pointer, this makes a lot more sense and also seems to be a lot simpler.

I added the GraphemeReplacement variant to the various style-related enums and a grapheme_replacement field to the various style-related structs. This way it should be (close to) usable from the render_text function in masonry_core, but I still have a few questions.

I am currently using Option<Arc<String>> for the replacement grapheme, mostly because I ran into some lifetime issues with &'static str, so I just wanted to check if this choice makes sense.

I also set grapheme_replacement to None in the test_builder.rs test. I wonder if this is right, or that I should set Some("*".to_string().into() instead. It currently fails if I set it to a non-default value.

Finally, I am also unsure about whether the output layout should still contain Option<Arc<String>> or whether that grapheme should be processed such that we can use draw_glyphs directly in masonry_core.

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.

3 participants