-
Notifications
You must be signed in to change notification settings - Fork 3.2k
docs(react): show complete code context in the "Your First App" tutorial #4201
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
base: main
Are you sure you want to change the base?
Conversation
|
@joesphchang is attempting to deploy a commit to the Ionic Team on Vercel. A member of the Team first needs to authorize it. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
thetaPC
left a comment
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 noticed that some method names from the Angular version and Vue version do not match even though they have the same code. For example, addNewToGallery from Angular and usePhotoGallery from Vue. Let's also sync those to match Angular as close as possible.
The same goes for the <title>. Let's match it to Angular.
Lastly, some of the content also doesn't match each other. A good example, Angular Build Your First App mentions:
`ion-header` represents the top navigation and toolbar, with "Tab 2" as the title (there are two of them due to iOS [Collapsible Large Title](https://ionicframework.com/docs/api/title#collapsible-large-titles) support). Rename both ion-title elements to:
While React states:
`IonHeader` represents the top navigation and toolbar, with "Tab 2" as the title. Let’s rename it:
Co-authored-by: Brandy Smith <6577830+brandyscarney@users.noreply.github.com>
brandyscarney
left a comment
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.
Left a comment with some optional changes, but looks good!
ShaneK
left a comment
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 had a few minor consistency nits/suggestions, but generally I think this looks great! Awesome work!
|
|
||
| ```tsx | ||
| ```ts | ||
| export function usePhotoGallery { |
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.
| export function usePhotoGallery { | |
| export function usePhotoGallery() { |
| import { Camera, CameraResultType, CameraSource } from '@capacitor/camera'; | ||
|
|
||
| When the camera is done taking a picture, the resulting Photo returned from Capacitor will be stored in the `photo` variable. We want to create a new photo object and add it to the photos state array. We make sure we don't accidentally mutate the current photos array by making a new array, and then call `setPhotos` to store the array into state. Update the `takePhoto` method and add this code after the getPhoto call: | ||
| export function usePhotoGallery { |
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.
| export function usePhotoGallery { | |
| export function usePhotoGallery() { |
|
|
||
| ```tsx | ||
| ```ts | ||
| export function usePhotoGallery { |
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.
| export function usePhotoGallery { | |
| export function usePhotoGallery() { |
Nit: I know this is just a reference point, but as a reference point we should still be accurate for the parts we do include
| import { Camera, CameraResultType, CameraSource } from '@capacitor/camera'; | ||
|
|
||
| When the camera is done taking a picture, the resulting Photo returned from Capacitor will be stored in the `photo` variable. We want to create a new photo object and add it to the photos state array. We make sure we don't accidentally mutate the current photos array by making a new array, and then call `setPhotos` to store the array into state. Update the `takePhoto` method and add this code after the getPhoto call: | ||
| export function usePhotoGallery { |
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.
| export function usePhotoGallery { | |
| export function usePhotoGallery() { |
| ``` | ||
|
|
||
| Next, let's expose the photos array from our hook. Update the return statement to include the photos: | ||
| Over in the `addNewToGallery()` method, add the newly captured photo to the beginning of the `photos` array. Then, update the `userPhotoGallery()` return statement with the `photos` array. |
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.
| Over in the `addNewToGallery()` method, add the newly captured photo to the beginning of the `photos` array. Then, update the `userPhotoGallery()` return statement with the `photos` array. | |
| Over in the `addNewToGallery()` method, add the newly captured photo to the beginning of the `photos` array. Then, update the `usePhotoGallery()` return statement with the `photos` array. |
Nit: Minor typo in method name
| ``` | ||
|
|
||
| Next, let's expose the photos array from our hook. Update the return statement to include the photos: | ||
| Over in the `addNewToGallery()` method, add the newly captured photo to the beginning of the `photos` array. Then, update the `userPhotoGallery()` return statement with the `photos` array. |
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.
| Over in the `addNewToGallery()` method, add the newly captured photo to the beginning of the `photos` array. Then, update the `userPhotoGallery()` return statement with the `photos` array. | |
| Over in the `addNewToGallery()` method, add the newly captured photo to the beginning of the `photos` array. Then, update the `usePhotoGallery()` return statement with the `photos` array. |
Nit: Minor typo in method name
| ``` | ||
|
|
||
| Next, add a state value to store information about the photo to delete: | ||
| Next, in `Tab2.tsx`, implement the `IonActionSheet` component. We're adding two options: "Delete", which calls `usePhotoGallery.deletePicture()`, and "Cancel". The cancel button will automatically close the action sheet when assigned the "cancel" role. |
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.
| Next, in `Tab2.tsx`, implement the `IonActionSheet` component. We're adding two options: "Delete", which calls `usePhotoGallery.deletePicture()`, and "Cancel". The cancel button will automatically close the action sheet when assigned the "cancel" role. | |
| Next, in `Tab2.tsx`, implement the `IonActionSheet` component. We're adding two options: "Delete", which calls `usePhotoGallery.deletePhoto()`, and "Cancel". The cancel button will automatically close the action sheet when assigned the "cancel" role. |
Nit: The exposed method name is incorrectly identified here
| ``` | ||
|
|
||
| Next, add a state value to store information about the photo to delete: | ||
| Next, in `Tab2.tsx`, implement the `IonActionSheet` component. We're adding two options: "Delete", which calls `usePhotoGallery.deletePicture()`, and "Cancel". The cancel button will automatically close the action sheet when assigned the "cancel" role. |
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.
| Next, in `Tab2.tsx`, implement the `IonActionSheet` component. We're adding two options: "Delete", which calls `usePhotoGallery.deletePicture()`, and "Cancel". The cancel button will automatically close the action sheet when assigned the "cancel" role. | |
| Next, in `Tab2.tsx`, implement the `IonActionSheet` component. We're adding two options: "Delete", which calls `usePhotoGallery.deletePhoto()`, and "Cancel". The cancel button will automatically close the action sheet when assigned the "cancel" role. |
Nit: The exposed method name is incorrectly identified here
| The selected photo is removed from the Photos array first. Then, we use the Capacitor Preferences API to update the cached version of the Photos array. Finally, we delete the actual photo file itself using the Filesystem API. | ||
|
|
||
| Make sure to return the `deletePhoto` function so it is as a part of the hook API that we expose: | ||
| Remember that removing the photo from the `photos` array triggers the `cachePhotos` method for us automatically. |
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.
| Remember that removing the photo from the `photos` array triggers the `cachePhotos` method for us automatically. | |
| Remember that removing the photo from the `photos` array triggers the `setPhotos` method for us automatically. |
Nit: cachePhotos isn't a method, I think this means to refer to setPhotos, which is called by delete and add new photos
| The selected photo is removed from the Photos array first. Then, we use the Capacitor Preferences API to update the cached version of the Photos array. Finally, we delete the actual photo file itself using the Filesystem API. | ||
|
|
||
| Make sure to return the `deletePhoto` function so it is as a part of the hook API that we expose: | ||
| Remember that removing the photo from the `photos` array triggers the `cachePhotos` method for us automatically. |
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.
| Remember that removing the photo from the `photos` array triggers the `cachePhotos` method for us automatically. | |
| Remember that removing the photo from the `photos` array triggers the `setPhotos` method for us automatically. |
Nit: cachePhotos isn't a method, I think this means to refer to setPhotos, which is called by delete and add new photos
Resolves: #4164
Changes:
Preview