-
Notifications
You must be signed in to change notification settings - Fork 535
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
Tutorial rework #3413
Tutorial rework #3413
Conversation
}; | ||
updateDiceChar(); | ||
} | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should quickly summarize the code above, in the first part of the function we define the various HTML elements we need and then we attach an event listener for when the button is clicked. Explain also why we need to define a function called updateDiceChar
and call it right away. This might be obvious but experienced people will skip the explanation and people surprised by this pattern won't get stuck on this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree we should summarize and/or add some inline comments to help chunk the code. Ie // create a new div that we will put our dice value in
... // create a new button that we will trigger a re-roll
... // function that generates a new value and updates the dice value
... ect.
|
||
*dataObject.ts* | ||
To clarify what our model needs to support, let's start by defining its public interface. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please explain what a public interface is. JS developers might have never heard of this term.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mattetti I think we have to assume that the readers have enough understanding of TypeScript to know what an interface is. If they don't, I think the tutorial will be beyond them. Let's put knowledge of TypeScript as a prerequisite at the top of the page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really think we should get rid of the interface layer. I get that it's proper typescript and OO programming but it's an extra layer of indirection that is not required to learn about Fluid.
The [full code for this application is available](https://github.com/microsoft/FluidHelloWorld) for you to try out. | ||
|
||
|
||
<!-- AUTO-GENERATED-CONTENT:START (INCLUDE:path=_includes/links.md) --> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please delete those comments, they are from the old system
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tylerbutler recommended these. Happy to change as needed but would like to get consensus first.
Once the application loads the container will communicate with the server to exchange DDS data: | ||
|
||
![](/docs/get-started/images/full-structure.png) | ||
|
||
The [full code for this application is available](https://github.com/microsoft/FluidHelloWorld) for you to try out. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great tutorial! I'd suggest to add a little summary at the end walking back the code from the last step to the beginning by following the click event the user will trigger.
|
||
*dataObject.ts* | ||
To clarify what our model needs to support, let's start by defining its public interface. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mattetti I think we have to assume that the readers have enough understanding of TypeScript to know what an interface is. If they don't, I think the tutorial will be beyond them. Let's put knowledge of TypeScript as a prerequisite at the top of the page.
|
||
|
||
### The container code | ||
### Defining the container contents |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we mention container in the header and talk about container code in this section, we have to stop here and define what a container is. Suggestion:
### Defining the container contents | |
### Defining the container contents | |
A **container** is a web application's entry point into the Fluid Framework. It contains all the DDSes and all the DataObject-derived objects. It also provides communication services that enable the app to report data changes to the other clients and receive data changes from them. |
DiceRoller. We can accomplish this using a [ContainerRuntimeFactoryWithDefaultDataStore][] -- this will create a | ||
single DiceRoller and make it available to be retrieved from the container. We'll provide it with the name of the | ||
default data object and a mapping of the name to factory. | ||
In our app, we only need a single instance of this single model for our single dice. However, in more complex scenarios we might have multiple model types with many model instances. The code you'll write to specify the type and number of data objects your application uses is the **container code**. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In our app, we only need a single instance of this single model for our single dice. However, in more complex scenarios we might have multiple model types with many model instances. The code you'll write to specify the type and number of data objects your application uses is the **container code**. | |
In our app, we only need a single instance of this single model for our single dice. However, in more complex scenarios we might have multiple model types with many model instances. The code you'll write to specify the type and number of data objects your application uses is the **container code**. This specification is called **registration**. |
|
||
*app.ts* | ||
For now, we'll just run on a local test service called [Tinylicious][], and to make this easier we've provided a helper function `getTinyliciousContainer()`. The helper function takes a unique ID to identify our document (the collection of data used by our app), the container code, and a flag to indicate whether we want to create a new document or load an existing one. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The function name getTinyliciousContainer
makes it sound like the container comes from the service. But the section title "Connect container to service for collaboration" implies that the container is something on the client that connects to the service. This is REALLY confusing. Can the function name be changed? If not, we need to acknowledge the confusion and explain the name. (Or simply state that the name is misleading.)
|
||
*app.ts* | ||
After we have the connected Container object, our container code will have already run to create an instance of our model. Since we built our container code using a ContainerRuntimeFactoryWithDefaultDataStore, the model can be requested from the Container object using a URL of "/".: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the first time we use "URL". We need to say what this URL is (or link to someplace where it is explained).
@@ -218,21 +201,9 @@ export function renderDiceRoller(diceRoller: IDiceRoller, div: HTMLDivEleme | |||
} | |||
``` | |||
|
|||
And then all that's left to do is render using the view: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now we should finish by telling the reader how to run the app in two windows and see how it works.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great improvement!
- **Container** -- The container is your application's entry point to Fluid Framework. It runs your container | ||
code and is the object through which you'll retrieve your data objects. | ||
1. Write the view | ||
1. Define the interface our model will expose |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm still not convinced this is an important step for hello world. It seems more like a learn TypeScript step as opposed to learn something important about the Fluid Framework.
1. Write the model using Fluid Framework | ||
1. Include our model in our container | ||
1. Connect our container to the service for collaboration | ||
1. Connect our model instance to our view for rendering |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we move #6 Connect our model instance to our view for rendering
to be #3.
I believe that view + model + connection is the core of what first run developers will care about. Once they have this they can start building themselves without feeling blocked. If they want to learn more about how to integrate this into a website learning about the container/application will be important. But until then I would love for developers to get to a natural stopping place that they feel they can explore before moving on.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately no - there's no way to get the model instance until we have a connected container we can request
on. Agreed that would be better though.
|
||
*dataObject.ts* | ||
To clarify what our model needs to support, let's start by defining its public interface. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really think we should get rid of the interface layer. I get that it's proper typescript and OO programming but it's an extra layer of indirection that is not required to learn about Fluid.
DiceRoller. We can accomplish this using a [ContainerRuntimeFactoryWithDefaultDataStore][] -- this will create a | ||
single DiceRoller and make it available to be retrieved from the container. We'll provide it with the name of the | ||
default data object and a mapping of the name to factory. | ||
In our app, we only need a single instance of this single model for our single dice. However, in more complex scenarios we might have multiple model types with many model instances. The code you'll write to specify the type and number of data objects your application uses is the **container code**. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've read this statement a few times trying to understand it. It's too generic to make sense if you don't understand that the container can host multiple models which you wouldn't at this point.
I guess the question I would ask is why do I need this container code if the only thing it does is expose a model? I think Rick's definition helps but it's still too many "single" and "multiple" in this sentence to make sense to someone who knows nothing.
After Rick's definition I would propose something like...
In our app, we only need a single instance of this single model for our single dice. However, in more complex scenarios we might have multiple model types with many model instances. The code you'll write to specify the type and number of data objects your application uses is the **container code**. | |
In our **container code** we will define the model we built previously by providing the type and the registry entry from our model factory. These two pieces of information allows our container code to create a connected instance of our model. |
![](/docs/get-started/images/container-code.png) | ||
|
||
*containerCode.ts* | ||
Since we only need a single dice, Fluid Framework provides a class called [ContainerRuntimeFactoryWithDefaultDataStore][] that we can use. This particular container code will instantiate a single instance of the model type we specify from the registry we provide. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea... This name is even worse in this context because it just raises so many questions... Like I'm building a container what's a ContainerRuntime??? Why is this also a Factory??? What's a default DataStore??? Actually... what's a DataStore???
![](/docs/get-started/images/container-code.png) | ||
|
||
*containerCode.ts* | ||
Since we only need a single dice, Fluid Framework provides a class called [ContainerRuntimeFactoryWithDefaultDataStore][] that we can use. This particular container code will instantiate a single instance of the model type we specify from the registry we provide. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
haha, I saw this after I typed my comment +100
|
||
*app.ts* | ||
For now, we'll just run on a local test service called [Tinylicious][], and to make this easier we've provided a helper function `getTinyliciousContainer()`. The helper function takes a unique ID to identify our document (the collection of data used by our app), the container code, and a flag to indicate whether we want to create a new document or load an existing one. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For now
seems like we will have something else in the doc... How about something more like... We have a local Fluid service called Tinylicious that we will be using locally
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do have something else, I mention the changes for production below.
I'd suggest we merge and deploy and open a new PR with refinements if needed |
Agreed - it sounds like there's general consensus that this sequencing and framing is easier to read than the current state so it's probably best to get it in and start iterating as needed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🍁 Huge improvement. Let's get it in and continue to iterate. :)
Reworking the tutorial based on feedback today. Opening as a draft because I intend to look at this more on Monday with fresh eyes, but want to give folks a preview of how the pivot feels.