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

docs: creating nested models #346

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

docs: creating nested models #346

wants to merge 7 commits into from

Conversation

bashbunni
Copy link
Member

Changes:

  • add info on working with nested models
  • add docs folder

@bashbunni bashbunni added the documentation Improvements or additions to documentation label Jun 17, 2022
@maaslalani
Copy link
Contributor

maaslalani commented Jul 1, 2022

I would suggest we should maybe make an example/nested-model instead as code is likely much easier to maintain than documentation as breaking API changes / evolution (i.e. introducing bubbles/navigation) comes it's very likely this example will be out of date in a few months but having that code not compile will make it easier to maintain and we can explain concepts in the comments of the example. What do you think @bashbunni?

@bashbunni
Copy link
Member Author

hey @maaslalani great suggestion. I actually found some typos while moving the code over to an example 😂 what do you think about these changes? I feel like these docs will become more substantial as we get more questions and as we get feedback. Just pushed the latest changes

@maaslalani
Copy link
Contributor

Looks awesome @bashbunni !!!

Copy link
Member

@meowgorithm meowgorithm left a comment

Choose a reason for hiding this comment

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

A few quick notes here:

  • Please do make sure the example code runs (I'm sure it's still a work-in-progress)
  • It would be great to show how one can use a state member in Update, in addition to View. For example, to change key handling when a section of the app changes focus, since that will be a super common need.

@bashbunni bashbunni force-pushed the nested-models-docs branch 2 times, most recently from fa23d0b to 41da988 Compare August 5, 2022 16:30
@bashbunni
Copy link
Member Author

tbd in dev meeting. I'd like some feedback on writing effective comments in the code and whether or not this example should use tea.Batch or return commands. I feel like returning commands makes the flow a bit more obvious, but using tea.Batch is the cleaner solution. I can make this PR a single commit as well if that's preferred.

@bashbunni bashbunni marked this pull request as draft August 5, 2022 20:19
@bashbunni bashbunni force-pushed the nested-models-docs branch 2 times, most recently from ec0f4a4 to d949506 Compare August 10, 2022 16:06
@bashbunni
Copy link
Member Author

This example no longer manages state from a main model. That is shown in the composable-view branch.

@bashbunni bashbunni marked this pull request as ready for review August 10, 2022 17:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants