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

initialize dev docs for loyalty extension #2486

Merged
merged 1 commit into from
Nov 28, 2024
Merged

Conversation

merkoyep
Copy link
Contributor

@merkoyep merkoyep commented Nov 21, 2024

Background

Accompanying PR
Resolves https://github.com/Shopify/pos-next-react-native/issues/42764

As part of the deprecating legacy extensions project, this PR aims to educate developers on how to create a loyalty extension following the deprecation of legacy UI Extensions. This PR will show developers create a tutorial that showcases how to build a loyalty extension using POS UI Extensions.

Solution

This PR will:

  • Add reference code examples for the loyalty extension tutorial.
  • Add a new directory (/docs/surfaces/point-of-sale/mdxExamples)that will hold all example code for mdx based code tutorials for POS UI Extensions
  • Add the mdxExamples folder to the array of folders to be ignored by eslint.

Video of how this code works:

LoyaltyExampleDemo.mp4.mov

🎩

  • n/a

Checklist

  • I have 🎩'd these changes
  • I have updated relevant documentation

@merkoyep merkoyep marked this pull request as ready for review November 21, 2024 22:16
Copy link
Contributor

@NathanJolly NathanJolly left a comment

Choose a reason for hiding this comment

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

Bravo. What a useful and thorough guide for our users.

@js-goupil
Copy link
Contributor

I noticed in the video, that the gif you provided has an RN error. We probably don't want to be surfacing any POS implementation details publicly, and it doesn't look as polished. Could we replace the gif with one recorded using an internal build, or even an app store build?

customerId: number,
setPointBalance: React.Dispatch<React.SetStateAction<number | null>>,
) => {
const [loading, setLoading] = useState(true);
Copy link
Contributor

@vctrchu vctrchu Nov 25, 2024

Choose a reason for hiding this comment

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

I recommend initializing the loading state to false. We don't want to have unnecessary loading states for cases where the extension doesn't need to fetch points. You can instead set it to true when we want to fetch data https://github.com/Shopify/ui-extensions/pull/2486/files#r1857180980

useEffect(() => {
const fetchOrderData = async () => {
try {
// Get the session token
Copy link
Contributor

@vctrchu vctrchu Nov 25, 2024

Choose a reason for hiding this comment

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

Suggested change
// Get the session token
setLoading(true)

Copy link
Contributor

@vctrchu vctrchu left a comment

Choose a reason for hiding this comment

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

You probably already have done this but running through a fresh extension and testing it by copy/pasting each block would be a great way to ensure no issues are apparent. Just a small comment around the loading state & +1 to nates pending comments but overall I'm very impressed from the detail and technically of this documentation, great job!

@merkoyep merkoyep force-pushed the my/loyaltyexampledocs branch 2 times, most recently from c1538c7 to b80f744 Compare November 25, 2024 20:09
@js-goupil
Copy link
Contributor

@merkoyep if we're doing static mdx pages in the apps section, I guess we don't need this PR anymore right?

@merkoyep merkoyep force-pushed the my/loyaltyexampledocs branch 2 times, most recently from 0885200 to 5e55341 Compare November 28, 2024 19:44
@merkoyep merkoyep force-pushed the my/loyaltyexampledocs branch from 265c811 to 09d099d Compare November 28, 2024 19:53
@merkoyep
Copy link
Contributor Author

@merkoyep if we're doing static mdx pages in the apps section, I guess we don't need this PR anymore right?

@js-goupil Not quite! Thanks to @vctrchu for helping me figure this out, but we are going to keep our example code files in this repo, so that I can reference this repo for the tutorial over on the shopify-dev side as code examples :)

@merkoyep merkoyep merged commit 38c3f8d into unstable Nov 28, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants