Fix newsletter subscription and UI modal #203
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
WalkthroughThis pull request introduces a more accessible and transition-friendly UI modal component using the Changes
Uplevel your code reviews with CodeRabbit ProCodeRabbit ProIf you like this project, please support us by purchasing the Pro version. The Pro version has advanced context, superior noise reduction and several proprietary improvements compared to the open source version. Moreover, CodeRabbit Pro is free for open source projects. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Files selected (3)
- gqty.config.cjs (1)
- src/components/Home/Modal.tsx (2)
- src/lib/subscribeUser.ts (1)
Files skipped from review due to trivial changes (1)
- gqty.config.cjs
Review comments generated (3)
- Review: 2
- LGTM: 1
Tips
Chat with 
CodeRabbit Bot (@coderabbitai)
- Reply on review comments left by this bot to ask follow-up questions. A review comment is a comment on a diff or a file.
- Invite the bot into a review comment chain by tagging
@coderabbitaiin a reply.
Code suggestions
- The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
- You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.
Pausing incremental reviews
- Add
@coderabbitai: ignoreanywhere in the PR description to pause further reviews from the bot.
| aria-hidden="true" | ||
| > | ||
| <path | ||
| strokeLinecap="round" | ||
| strokeLinejoin="round" | ||
| strokeWidth="2" | ||
| d="M5 13l4 4L19 7" | ||
| ></path> | ||
| </svg> | ||
| </div> | ||
| <div className="mt-3 px-5 text-center sm:mt-5"> | ||
| <p | ||
| className="text-lg font-medium leading-6 text-gray-900 dark:text-gray-200" | ||
| id="modal-title" | ||
| > | ||
| Subscription Successful! | ||
| </p> | ||
| <div className="mt-2"> | ||
| <p className="text-sm text-gray-500 dark:text-gray-300"> | ||
| Congratulations! You have now subscribed to the daily | ||
| "Shloka of the Day" newsletter. | ||
| </p> | ||
| <div className="fixed inset-0 z-10 overflow-y-auto"> | ||
| <div className="flex min-h-full items-end justify-center p-4 text-center sm:items-center sm:p-0"> | ||
| <Transition.Child | ||
| as={Fragment} | ||
| enter="ease-out duration-300" | ||
| enterFrom="opacity-0 translate-y-4 sm:translate-y-0 sm:scale-95" | ||
| enterTo="opacity-100 translate-y-0 sm:scale-100" | ||
| leave="ease-in duration-200" | ||
| leaveFrom="opacity-100 translate-y-0 sm:scale-100" | ||
| leaveTo="opacity-0 translate-y-4 sm:translate-y-0 sm:scale-95" | ||
| > | ||
| <Dialog.Panel className="relative overflow-hidden rounded-lg bg-white text-left shadow-xl transition-all sm:w-full sm:max-w-sm"> | ||
| <div> | ||
| <div className="z-50 inline-block overflow-hidden rounded-lg bg-white text-left align-bottom shadow-xl transition-all duration-200 dark:bg-dark-100 sm:mt-8 sm:w-full sm:max-w-sm sm:p-6 sm:align-middle"> | ||
| <div> | ||
| <div className="mx-auto flex h-12 w-12 items-center justify-center rounded-full bg-yellow-100 dark:bg-dark-bg "> | ||
| <svg | ||
| className="h-6 w-6 text-yellow-400" | ||
| x-description="Heroicon name: outline/check" | ||
| xmlns="http://www.w3.org/2000/svg" | ||
| fill="none" | ||
| viewBox="0 0 24 24" | ||
| stroke="currentColor" | ||
| aria-hidden="true" | ||
| > | ||
| <path | ||
| strokeLinecap="round" | ||
| strokeLinejoin="round" | ||
| strokeWidth="2" | ||
| d="M5 13l4 4L19 7" | ||
| ></path> | ||
| </svg> | ||
| </div> | ||
| <div className="mt-3 px-5 text-center sm:mt-5"> | ||
| <p | ||
| className="text-lg font-medium leading-6 text-gray-900 dark:text-gray-200" | ||
| id="modal-title" | ||
| > | ||
| Subscription Successful! | ||
| </p> | ||
| <div className="mt-2"> | ||
| <p className="text-sm text-gray-500 dark:text-gray-300"> | ||
| Congratulations! You have now subscribed to the | ||
| daily "Shloka of the Day" newsletter. | ||
| </p> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| <div className="mt-5 sm:mt-6"> | ||
| <button | ||
| type="button" | ||
| className="mx-auto flex w-1/3 justify-center rounded-md border border-transparent bg-my-orange px-4 py-2 text-base font-medium text-white shadow-sm hover:bg-opacity-75 focus:outline-none focus:ring-2 focus:ring-yellow-500 focus:ring-offset-2 sm:text-sm" | ||
| onClick={() => setModalVisible(false)} | ||
| > | ||
| I got it | ||
| </button> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| <div className="mt-5 sm:mt-6"> | ||
| <button | ||
| type="button" | ||
| className="mx-auto flex w-1/3 justify-center rounded-md border border-transparent bg-my-orange px-4 py-2 text-base font-medium text-white shadow-sm hover:bg-opacity-75 focus:outline-none focus:ring-2 focus:ring-yellow-500 focus:ring-offset-2 sm:text-sm" | ||
| onClick={() => setModalVisible(false)} | ||
| > | ||
| I got it | ||
| </button> | ||
| </div> | ||
| </Dialog.Panel> | ||
| </Transition.Child> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| </Dialog> | ||
| </Transition.Root> |
There was a problem hiding this comment.
The new implementation of the modal using @headlessui/react is a good improvement in terms of accessibility and transition effects. However, I noticed that you are not setting an initial focus when the modal opens which is important for accessibility. The Dialog component from @headlessui/react provides a staticInitialFocus prop that can be used to set an initial focus on an element when the dialog opens.
Here's how you can add it:
<Dialog as="div" className="relative z-10" onClose={setModalVisible}>
+ <Dialog as="div" className="relative z-10" onClose={setModalVisible} staticInitialFocus>This will ensure that the focus is trapped within the modal for better keyboard navigation.
| return resolved(() => { | ||
| const result = mutation.insert_newsletter_subscriptions_one({ | ||
| object: { | ||
| user_name: name, | ||
| user_email: email, | ||
| }, | ||
| }), | ||
| ); | ||
| }); | ||
|
|
||
| return result?.id; | ||
| }); |
There was a problem hiding this comment.
The changes made to the subscribeUser function are good for improving readability and error handling. However, it's important to note that the resolved function from gqty-client is a synchronous function. Wrapping it with async keyword might be misleading as it suggests that the function returns a Promise which is not the case here.
- export const subscribeUser = async (name: string, email: string) => {
+ export const subscribeUser = (name: string, email: string) => {Also, the result?.id could potentially return undefined if the mutation operation fails. It would be better to handle this case explicitly.
- return result?.id;
+ if (result) {
+ return result.id;
+ } else {
+ throw new Error('Subscription failed');
+ }
Summary by CodeRabbit
gqtyby removing an unnecessary error throw related to theNEXT_PUBLIC_GRAPHQL_ENDPOINTenvironment variable.@headlessui/reactlibrary.idproperty of the mutation result, providing more detailed feedback on the subscription process.