-
Notifications
You must be signed in to change notification settings - Fork 5.1k
Update apple-reminders extension #24388
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
- feat: open all attached reminder URLs - feat: add action to open attached reminder URLs
|
Thank you for your contribution! 🎉 🔔 @thomaslombart @tmwrnr @irangarcia @jondelga @phil1995 @michalzuch @ridemountainpig @maxnyby @vmrjnvc @ramith123 you might want to have a look. You can use this guide to learn how to check out the Pull Request locally in order to test it. 📋 Quick checkout commandsBRANCH="ext/apple-reminders"
FORK_URL="https://github.com/ScH01ar/raycast-extensions.git"
EXTENSION_NAME="apple-reminders"
REPO_NAME="raycast-extensions"
git clone -n --depth=1 --filter=tree:0 -b $BRANCH $FORK_URL
cd $REPO_NAME
git sparse-checkout set --no-cone "extensions/$EXTENSION_NAME"
git checkout
cd "extensions/$EXTENSION_NAME"
npm install && npm run devDue to our current reduced availability, the initial review may take up to 10-15 business days. |
Greptile OverviewGreptile SummaryThis PR successfully implements URL extraction and batch opening functionality for Apple Reminders. The implementation: What Changed:
Technical Implementation:
Areas for Improvement: Additionally, the CHANGELOG description mentions "the first URL" when the implementation actually extracts and opens all URLs found. Confidence Score: 3/5
Important Files ChangedFile Analysis
|
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.
3 files reviewed, 3 comments
|
|
||
| ## [Add action to open attached URL] - {PR_MERGE_DATE} | ||
|
|
||
| - Detect the first URL in reminder notes (or Reminder URL when available) and expose an “Open Attached URL” action in the list and menu bar. |
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.
The CHANGELOG description says "Detect the first URL in reminder notes" but the implementation actually extracts and allows opening all URLs found in the reminder (both from EKReminder.url and all URLs parsed from notes).
Consider updating to: "Detect all URLs in reminder notes (and Reminder URL when available) and expose an 'Open Attached URL(s)' action in the list and menu bar."
| onAction={async () => { | ||
| for (const url of attachedUrls) { | ||
| await open(url); | ||
| } | ||
| }} |
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.
The loop opening multiple URLs lacks error handling. If opening one URL fails, subsequent URLs may not open and the user gets no feedback about what went wrong.
Other actions in this file (like toggleReminder, setPriority) wrap their operations in try-catch blocks with toast notifications. Consider adding similar error handling here:
onAction={async () => {
try {
for (const url of attachedUrls) {
await open(url);
}
if (attachedUrls.length > 1) {
await showToast({
style: Toast.Style.Success,
title: `Opened ${attachedUrls.length} URLs`,
});
}
} catch (error) {
await showToast({
style: Toast.Style.Failure,
title: "Unable to open URL(s)",
});
}
}}Reference: The search-router extension (extensions/search-router/src/search.tsx) uses this pattern when opening multiple URLs.
| onAction={async () => { | ||
| for (const url of attachedUrls) { | ||
| await open(url); | ||
| } | ||
| }} |
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.
Similar to ReminderActions.tsx, this URL opening loop lacks error handling. If opening a URL fails, the user receives no feedback.
Consider wrapping in try-catch with toast notifications for consistency with other operations in this file (see setPriority, setDueDate, deleteReminder which all have proper error handling).
Additionally, when opening multiple URLs, consider showing a success toast to confirm the action completed (see the search-router extension for reference: extensions/search-router/src/search.tsx lines 40-46).
0xdhrv
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.
Hi @ScH01ar 👋
Thanks for your contribution 💪
Could you look into the suggestions from Greptile
I'm looking forward to reviwing this extension again 🔥
I converted this PR into a draft until it's ready for the review, please press the button Ready for review when it's ready and we'll have a look 😊
Feel free to contact me here or at Slack if you have any questions.
Description
Resolve #24221: Extract and open all URLs from Reminders.
This PR improves the Apple Reminders integration by surfacing all attached URLs:
Screencast
npm run buildand tested this distribution build in Raycastassetsfolder are used by the extension itselfREADMEare placed outside of themetadatafolder