Add “Open in Reader” button to open generated RSS feeds in feed readers#31
Closed
DisabledAbel wants to merge 4 commits into
Closed
Add “Open in Reader” button to open generated RSS feeds in feed readers#31DisabledAbel wants to merge 4 commits into
DisabledAbel wants to merge 4 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThis pull request adds an "Open in Reader" feature to three HTML files (api, index, and templates versions). The feature enables users to open selected YouTube RSS feeds in external reader applications by converting http(s) protocol URLs to feed:// protocol and opening them in a new browser tab. ChangesOpen in Reader Feature
Possibly related PRs
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (4)
api/index.html (2)
183-192: ⚡ Quick winConsider error handling for popup blockers.
The
window.opencall may be blocked by popup blockers or fail if no feed:// protocol handler is registered. Users won't receive feedback in these cases.💡 Add error handling and user feedback
function openInReader(encodedText) { const feedUrl = decodeURIComponent(encodedText); const feedProtocolUrl = feedUrl.startsWith('https://') ? 'feed://' + feedUrl.slice('https://'.length) : feedUrl.startsWith('http://') ? 'feed://' + feedUrl.slice('http://'.length) : feedUrl; - window.open(feedProtocolUrl, '_blank', 'noopener,noreferrer'); + const opened = window.open(feedProtocolUrl, '_blank', 'noopener,noreferrer'); + if (!opened || opened.closed || typeof opened.closed === 'undefined') { + showToast('Could not open feed reader. Please check your popup blocker or install a feed reader app.', true); + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@api/index.html` around lines 183 - 192, The openInReader function currently calls window.open(feedProtocolUrl, '_blank', 'noopener,noreferrer') without handling failures; update openInReader to catch exceptions and check the return value (popup window handle) from window.open, and if it is null or an exception occurs show user feedback (e.g., call a UI helper like showErrorToast or alert) explaining the open failed and provide a fallback (copy feedProtocolUrl to clipboard or display the decoded feedUrl so the user can manually open it); ensure you reference the existing symbols openInReader, feedProtocolUrl and the window.open call when implementing the checks and fallback.
242-243: ⚡ Quick winInconsistent event handling pattern.
These lines use inline
onclickhandlers, but the file already has event delegation for copy buttons (lines 292-299) usingdata-encodedattributes. Line 253 also usesdata-encodedfor official feeds. This inconsistency makes the code harder to maintain.♻️ Refactor to use consistent event delegation pattern
Change the HTML generation to use data-encoded attributes:
-html += '<button class="copy-btn" onclick="copyEncodedText(\'' + encodeURIComponent((data.selected_feed || data.youtube_rss)) + '\')">Copy Selected RSS</button>'; -html += '<button class="open-reader-btn" onclick="openInReader(\'' + encodeURIComponent((data.selected_feed || data.youtube_rss)) + '\')">Open in Reader</button></div>'; +html += '<button class="copy-btn" data-encoded="' + encodeURIComponent((data.selected_feed || data.youtube_rss)) + '">Copy Selected RSS</button>'; +html += '<button class="open-reader-btn" data-encoded="' + encodeURIComponent((data.selected_feed || data.youtube_rss)) + '">Open in Reader</button></div>';Add event delegation for open-reader-btn (after line 299):
// Event delegation for dynamically added open reader buttons document.addEventListener('click', function(e) { if (e.target && e.target.classList.contains('open-reader-btn')) { const encoded = e.target.getAttribute('data-encoded'); if (encoded) { openInReader(encoded); } } });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@api/index.html` around lines 242 - 243, The generated buttons use inline onclick handlers instead of the existing data-driven delegation pattern; update the HTML string that builds the buttons to set data-encoded="..." on both the copy and open buttons (remove the onclick attributes) and keep the classes 'copy-btn' and 'open-reader-btn', then add an event-delegation listener that detects clicks on elements with class 'open-reader-btn', reads the data-encoded value and calls openInReader(encoded) (mirror the existing delegation used for copy buttons which calls copyEncodedText). Ensure you reference the same attribute name 'data-encoded' and the functions copyEncodedText and openInReader so behavior remains identical.index.html (1)
179-187: ⚡ Quick winConsider error handling for popup blockers.
The
window.opencall may be blocked by popup blockers or fail if no feed:// protocol handler is registered.💡 Add error handling and user feedback
function openInReader(feedUrl) { const feedProtocolUrl = feedUrl.startsWith('https://') ? 'feed://' + feedUrl.slice('https://'.length) : feedUrl.startsWith('http://') ? 'feed://' + feedUrl.slice('http://'.length) : feedUrl; - window.open(feedProtocolUrl, '_blank', 'noopener,noreferrer'); + const opened = window.open(feedProtocolUrl, '_blank', 'noopener,noreferrer'); + if (!opened || opened.closed || typeof opened.closed === 'undefined') { + showToast('Could not open feed reader. Please check your popup blocker or install a feed reader app.', true); + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@index.html` around lines 179 - 187, The openInReader function needs popup-block and failure handling: after building feedProtocolUrl, call window.open and check its return value (the opened window reference) and wrap the call in try/catch; if window.open returns null/undefined or throws, show user feedback (e.g., display an in-page message or alert) explaining the open failed and provide a fallback link or instructions to copy the feed URL (use the feedProtocolUrl string). Ensure you reference the feedProtocolUrl and window.open call inside openInReader and surface a clear, user-friendly error message.templates/index.html (1)
178-189: ⚡ Quick winConsider error handling for popup blockers.
The
window.opencall may be blocked by popup blockers or fail if no feed:// protocol handler is registered.💡 Add error handling and user feedback
function openInReader(encodedText) { const feedUrl = decodeURIComponent(encodedText); // Many desktop/mobile feed apps register the feed:// protocol. // Convert https:// URLs to feed:// where possible to trigger RSS readers. const feedProtocolUrl = feedUrl.startsWith('https://') ? 'feed://' + feedUrl.slice('https://'.length) : feedUrl.startsWith('http://') ? 'feed://' + feedUrl.slice('http://'.length) : feedUrl; - window.open(feedProtocolUrl, '_blank', 'noopener,noreferrer'); + const opened = window.open(feedProtocolUrl, '_blank', 'noopener,noreferrer'); + if (!opened || opened.closed || typeof opened.closed === 'undefined') { + showToast('Could not open feed reader. Please check your popup blocker or install a feed reader app.', true); + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@templates/index.html` around lines 178 - 189, The openInReader function should handle failures from window.open (popup blockers or missing feed:// handler): wrap the window.open call in a try/catch and check its return value; if window.open returns null or an exception is thrown, fallback by attempting to open the original feedUrl (e.g., set window.location.href) or show a user-facing message/alert via your app’s UI indicating the reader could not be launched and suggesting manual subscription; reference openInReader, feedProtocolUrl and feedUrl when adding these checks and the fallback/user-feedback path.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@api/index.html`:
- Around line 183-192: The openInReader function currently calls
window.open(feedProtocolUrl, '_blank', 'noopener,noreferrer') without handling
failures; update openInReader to catch exceptions and check the return value
(popup window handle) from window.open, and if it is null or an exception occurs
show user feedback (e.g., call a UI helper like showErrorToast or alert)
explaining the open failed and provide a fallback (copy feedProtocolUrl to
clipboard or display the decoded feedUrl so the user can manually open it);
ensure you reference the existing symbols openInReader, feedProtocolUrl and the
window.open call when implementing the checks and fallback.
- Around line 242-243: The generated buttons use inline onclick handlers instead
of the existing data-driven delegation pattern; update the HTML string that
builds the buttons to set data-encoded="..." on both the copy and open buttons
(remove the onclick attributes) and keep the classes 'copy-btn' and
'open-reader-btn', then add an event-delegation listener that detects clicks on
elements with class 'open-reader-btn', reads the data-encoded value and calls
openInReader(encoded) (mirror the existing delegation used for copy buttons
which calls copyEncodedText). Ensure you reference the same attribute name
'data-encoded' and the functions copyEncodedText and openInReader so behavior
remains identical.
In `@index.html`:
- Around line 179-187: The openInReader function needs popup-block and failure
handling: after building feedProtocolUrl, call window.open and check its return
value (the opened window reference) and wrap the call in try/catch; if
window.open returns null/undefined or throws, show user feedback (e.g., display
an in-page message or alert) explaining the open failed and provide a fallback
link or instructions to copy the feed URL (use the feedProtocolUrl string).
Ensure you reference the feedProtocolUrl and window.open call inside
openInReader and surface a clear, user-friendly error message.
In `@templates/index.html`:
- Around line 178-189: The openInReader function should handle failures from
window.open (popup blockers or missing feed:// handler): wrap the window.open
call in a try/catch and check its return value; if window.open returns null or
an exception is thrown, fallback by attempting to open the original feedUrl
(e.g., set window.location.href) or show a user-facing message/alert via your
app’s UI indicating the reader could not be launched and suggesting manual
subscription; reference openInReader, feedProtocolUrl and feedUrl when adding
these checks and the fallback/user-feedback path.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 872d1452-5d04-41aa-b669-ce5c18c88958
📒 Files selected for processing (3)
api/index.htmlindex.htmltemplates/index.html
Co-authored-by: devin-ai-integration[bot] <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
feed://handler.Description
openInReaderclient-side helper that convertshttp:///https://feed URLs intofeed://where possible and opens them withwindow.open.Open in Readeraction button and event wiring next to the existing copy action for the selected feed in the UI..open-reader-btnstyling to keep the new action visually aligned with the existing controls.api/index.html,index.html, andtemplates/index.htmlso all served frontends behave the same.Testing
python -m py_compile api/app.py rss_scanner.pywhich completed successfully./returned200and the served HTML containsOpen in Readerandfunction openInReader, which succeeded.Codex Task
Summary by CodeRabbit