search

Church-Life-Apps / SongsV2

Hymnal App V2
1 stars 0 forks source link

started creating the SheetMusicComponent #30

Closed Daniel-727 closed 2 months ago

brandonxia01 commented 1 year ago

Will review in a bit!

For the next iteration of this PR and for future PRs, can you do a few things?

  1. Add some information to the description box of what is contained in this PR, and screenshots of what it does would be great too. You can check out some of my backend PRs as examples - https://github.com/Church-Life-Apps/Backend/pull/18

Similarly, make the title of the PR nice (succinct, descriptive, start with a capital letter)

  1. If the PR itself is not fully complete, you can open it up as a draft PR and I can still review it as normal, it just lets other developers know it's so a work in progress which is totally fine

  2. Post in the Discord when you put up a PR so others can see it too!

Thank you!

brandonxia01 commented 1 year ago

@Daniel-727 can you add some info in the description or post a comment about the status of this PR?

Daniel-727 commented 1 year ago

@Daniel-727 can you add some info in the description or post a comment about the status of this PR?

Yeah so I updated the code according to your comments. The sheet music shows up in the web version but not in the app. Also, pinch and zoomable haven't been implemented yet. I'm looking into react native gesture handler library from expo to handle that.

Daniel-727 commented 1 year ago

Oh some more details on the image not showing up in the app. It shows up, but is cut off when you use "cover", or "stretch" for the resizeMode. It doesn't show up when you use "contain".

brandonxia01 commented 1 year ago

@Daniel-727 can you add some info in the description or post a comment about the status of this PR?

Yeah so I updated the code according to your comments. The sheet music shows up in the web version but not in the app. Also, pinch and zoomable haven't been implemented yet. I'm looking into react native gesture handler library from expo to handle that.

Cool, sounds good. Yeah gesture handler sounds right. Might be worth it too to check out online to see if theres any external library you can try to use which has what we need implemented already and it might be easier than coding it up ourselves. But also feel free to focus on just getting the basic image working first, and saving zoomable for another pr. Let me know if you need help at any point, I'm open to taking a stab at the code.

Daniel-727 commented 1 year ago

@brandonxia01 so the image now shows up on mobile, and should be responsive on all devices. Using the developer tools it seems like it is responsive and looks fine; however, on my phone it's not quite centered and part of the image is cut off on the right