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

Form States #31

Merged
merged 2 commits into from
Nov 7, 2023
Merged

Form States #31

merged 2 commits into from
Nov 7, 2023

Conversation

maaslalani
Copy link
Contributor

Form state for handling quits in bubbletea applications.

@caarlos0
Copy link
Member

caarlos0 commented Nov 7, 2023

this works, but as mentioned in #30, requires the caller to wraps the form.

I'm good with either way you decide to impl this btw

@caarlos0
Copy link
Member

caarlos0 commented Nov 7, 2023

tried it here: https://github.com/charmbracelet/wishlist-internal/pull/33/files

I don't think its possible to get the state of the form inside Update after it finished, though... I think its because the default command is nil instead of tea.Quit, so Update is not called anymore (?)

@caarlos0
Copy link
Member

caarlos0 commented Nov 7, 2023

if we fix this particular issue, I don't think we even need #32

@caarlos0
Copy link
Member

caarlos0 commented Nov 7, 2023

maybe instead of having form.State, return a command that returns either a FormComplete or FormAborted tea.Msg? 🤔

@maaslalani
Copy link
Contributor Author

maybe instead of having form.State, return a command that returns either a FormComplete or FormAborted tea.Msg? 🤔

This would be quite simple and nice to use but not very Bubble Tea like since the Cmds and Msgs should be reserved for I/O rather than state changes. If it's possible to simply check the state rather than send a message we should probably do that which makes everything easier to test and more mechanical like @meowgorithm was mentioning.

@caarlos0
Copy link
Member

caarlos0 commented Nov 7, 2023

hmm, yeah, I agree, this should be fine.

@maaslalani maaslalani merged commit 9250f9d into main Nov 7, 2023
@maaslalani maaslalani deleted the without-quit branch November 7, 2023 19:51
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