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

Feature requests #42

Open
milesj opened this issue Dec 13, 2024 · 9 comments
Open

Feature requests #42

milesj opened this issue Dec 13, 2024 · 9 comments
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@milesj
Copy link

milesj commented Dec 13, 2024

Just going to note all my requests/ideas here:

1) Rename Box to View

Self-explanatory. Just collides with Rust native Box.

2) Support a Handler with a return value

I'm working on form/input components and I'm adding a validate function kind of prop, which works. https://github.com/moonrepo/starbase/blob/console/crates/console/src/components/input.rs#L13

However, I had to basically copy all of the Handler code to make it work, as I needed a return type of Option<String> (invalid with error message or valid). https://github.com/moonrepo/starbase/blob/console/crates/console/src/components/mod.rs#L29

Would be nice to have a built-in type for a handler with a return function. Adding it to the existing Handler may be too much, as we would then need to return () everywhere or something. So a new type maybe best.

3) Prop type that can be a String or AnyElement

I've found myself needing this pattern a lot just for convenience. Basically I want something like this:

struct ExampleProps {
  label: StringOrElement
}

So that when we're rendering, we can use a simple string (which would just render Text under the hood):

Example(label: &some_string_value)

Or we can customize with a specific element:

Example(label: element!(CustomLabel).into_any())
@ccbrown ccbrown added enhancement New feature or request good first issue Good for newcomers labels Dec 14, 2024
@milesj
Copy link
Author

milesj commented Dec 17, 2024

4) render_loop without ANSI

It looks like render_loop always renders with ANSI, even when in CI and non TTY environments, as seen here: https://github.com/moonrepo/proto/actions/runs/12342638090/job/34442711429?pr=671 Would be nice if this was turned off.

@ccbrown
Copy link
Owner

ccbrown commented Dec 17, 2024

4) render_loop without ANSI

It looks like render_loop always renders with ANSI, even when in CI and non TTY environments, as seen here: https://github.com/moonrepo/proto/actions/runs/12342638090/job/34442711429?pr=671 Would be nice if this was turned off.

The trick here is that render_loop can't function correctly without using ANSI sequences to erase prior output. A way to disable colors / text attributes would be reasonable though.

Also, for testing purposes, mock_terminal_render_loop might meet your needs – it allows you to get output as a sequence of Canvass which you can then print or turn into a string without ANSI codes.

@milesj
Copy link
Author

milesj commented Dec 17, 2024

Yeah that makes sense. Was mainly referring to all the color related sequences.

@milesj
Copy link
Author

milesj commented Dec 20, 2024

5) Support non-primitive values in use_state

I have a situation where I want to use a HashSet in state, but I have no way of inserting a value into the set after it's created. get requires Copy, which sets aren't, while read is immutable. The modify function would work, but it's not public.

@ccbrown
Copy link
Owner

ccbrown commented Dec 20, 2024

5) Support non-primitive values in use_state

I have a situation where I want to use a HashSet in state, but I have no way of inserting a value into the set after it's created. get requires Copy, which sets aren't, while read is immutable. The modify function would work, but it's not public.

I just added a write function in #45 (merged to main). Give that a try and let me know if that suits your needs. With that, you should be able to do something like my_state.write().insert(foo).

Thanks as always for the excellent feedback!

@milesj
Copy link
Author

milesj commented Dec 20, 2024

Yeah that worked. Thanks for the quick turn around.

I did encounter another bug though. It looks like ExtendWithElements::extend collides with the native vec/map/set extend. May need to rename that method.

Screenshot 2024-12-19 at 11 34 37 PM

@ccbrown
Copy link
Owner

ccbrown commented Dec 20, 2024

I did encounter another bug though. It looks like ExtendWithElements::extend collides with the native vec/map/set extend. May need to rename that method.

I might also want to exclude that trait from the prelude, but I've renamed the method for now and released it along with the state.write function in 0.5.1.

@milesj
Copy link
Author

milesj commented Dec 27, 2024

6) Support left/right arrows + caret in TextInput

This is a nice to have. Noticed while implementing inputs that TextInput doesn't allow left/right arrows, so you have to backspace to fix any issues. Would be nice to have this implemented natively. I tried to abstract around it, but since TextInput uses push internally, it won't work correctly.

It would also be nice to render the "caret" of where the current position is. Probably with just a bg color.

If I have time, I can look into contributing this.

@ccbrown
Copy link
Owner

ccbrown commented Dec 30, 2024

After experiencing more pain around number 1 here, I've decided it's time to bite the bullet and do the rename from Box to View. It's better to do it sooner rather than later, and I don't know of any other upcoming breaking changes that it could be batched with.

This is merged to main (2813e7992deec4aa68c1c6365bafb4617d200d2b) and will be released shortly as 0.6.0.

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

No branches or pull requests

2 participants