ShopPage Component example #220
Conversation
Add cooler demo items image
| local ReplicatedStorage = game:GetService("ReplicatedStorage") | ||
| local Players = game:GetService("Players") | ||
|
|
||
| local Components = ReplicatedStorage:WaitForChild("Components") |
There was a problem hiding this comment.
It's not mentioned beforehand that you should store ShopPage in ReplicatedStorage.Components. Would be good to clear that up.
There was a problem hiding this comment.
I added a comment above to try to make it clearer, if you think of anything else we could do let me know! Or you can submit a suggestion too!
| ```lua | ||
| function ProductItem:willUnmount() | ||
| self.toBigIcon:Destroy() | ||
| self.toNormalIcon:Destroy() |
There was a problem hiding this comment.
Do Tweens actually have to be destroyed? Given that they're only stored in memory they should be cleaned up regardless when ProductItem is unmounted. They also don't seem like they have a Destroy method
There was a problem hiding this comment.
I don't think you can find the info on the devhub. It does not show the ancestor functions and I don't really understand how I am suppose to know what are the ancestors of the class I am looking at. In fact, it's not really clear if it's an Instance
Anyway, you can check the ancestors on rodocs and yes apparently it's a descendant of Instance.
But should we still destroy it? I wanted to have an example of clean up you can do in the willUnmount lifecycle method.
docs/tutorials/shop-page.md
Outdated
| > On the other hand, props are used to pass data from parent components to child components. It implies that both parent component and it's child are aware of the structure of the props dictionary and depends on it. | ||
|
|
||
| For our first draft of the component, we will define some props to be able to pass parameters to customize our shop page. We will start by adding the following: | ||
| - **frameProps:** a dictionary of props that will be passed to the scrolling frame to change it's visuals |
There was a problem hiding this comment.
Could frameProps be removed? It seems strange to include as changing the style of the ScrollingFrame isn't part of what we want to be teaching. It seemed like you were modifying frameProps to change the CanvasSize but could that be handled differently without bloating the tutorial?
There was a problem hiding this comment.
My goal was to make the frame style accessible from the top level component, so someone could easily modify it from the script used to mount the component to fit his style
Having it also allow to tell the reader that the props cannot be mutated in the ShopPage component.
How would you change it?
Revise wording so things flow better
Rename ProductItem to Item and ProductItemList to ItemList Fix aspect ratio
| identifier = "Red Visor", | ||
| image = "https://www.roblox.com/asset-thumbnail/image?assetId=139618072&width=420&height=420&format=png", | ||
| price = 30, | ||
| productId = 0, |
There was a problem hiding this comment.
should we use real productIds and use those as the stable keys for the list items so we can get rid of the identifier field?
There was a problem hiding this comment.
Hum I could put random values... Even if we keep the 0 values I could use the list index as a key and get rid of the identifier.
I think at first my plan was to show the name identifier in a label but I dropped that.
|
|
||
| local Roact = require(ReplicatedStorage.Roact) | ||
|
|
||
| local Item = require(Components:WaitForChild("Item")) |
There was a problem hiding this comment.
Why do we need WaitForChild here? Is it unsafe for children of a folder to assume its siblings are available?
There was a problem hiding this comment.
I don't think there is any guarantee that all the siblings are replicated. From my experience it's best to use WaitForChild since you're sure everything is there when you need it.
Maybe @LPGhatguy can validate this!
This example aims to integrate some of Roact concepts together in a more realistic component that could exists in a Roblox game.
I make this draft pull request so anyone can already review the code itself to see if there are any misleading stuff going on that could be confusing or considered bad practice.