Skip to content

Conversation

@salmanfarisvp
Copy link
Collaborator

@salmanfarisvp salmanfarisvp commented Apr 30, 2025

User description

Access Privated Passcode enabled - Klipfolio Power Metrics Dashboard over Edge App.

image

ToDo:

  • Fix CORS Issue
  • Fix IFrame issue
  • Adde Sentry for logs

PR Type

Enhancement


Description

Add Klipfolio dashboard HTML with iframe
Implement passcode injection and proxy logic
Provide full-screen CSS styling
Add Screenly manifest and instance configurations


Changes walkthrough 📝

Relevant files
Formatting
styles.css
Add full-screen styling CSS                                                           

edge-apps/klipfolio-power-metrics-dashboard/styles.css

  • Added placeholder CSS file for reference
  • Defined full-screen body and html styles
  • Styled iframe to occupy full screen
  • +17/-0   
    Enhancement
    script.js
    Implement iframe URL and passcode injection                           

    edge-apps/klipfolio-power-metrics-dashboard/script.js

  • Initialize dashboard iframe using settings
  • Use Screenly CORS proxy or fallback URL
  • Inject passcode into iframe input and submit
  • Handle load timing and cross-origin errors
  • +68/-0   
    index.html
    Add HTML skeleton with iframe                                                       

    edge-apps/klipfolio-power-metrics-dashboard/index.html

  • Added HTML skeleton for Klipfolio dashboard
  • Included iframe and linked script and CSS
  • +14/-0   
    Configuration changes
    instance.yml
    Add app instance configuration                                                     

    edge-apps/klipfolio-power-metrics-dashboard/instance.yml

  • Added instance configuration for edge app
  • Defined syntax, id, and name fields
  • +4/-0     
    screenly.yml
    Add Screenly manifest settings                                                     

    edge-apps/klipfolio-power-metrics-dashboard/screenly.yml

  • Defined Screenly manifest for edge app
  • Configured dashboard_url and passcode settings
  • +15/-0   

    Need help?
  • Type /help how to ... in the comments thread for any questions about PR-Agent usage.
  • Check out the documentation for more information.
  • @github-actions
    Copy link

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Cross-Origin Access

    Injecting content into an external iframe will fail under cross-origin policy. Ensure the dashboard and host share the same origin or use a messaging API.

    let iframeDoc;
    try {
        iframeDoc = iframe.contentDocument || iframe.contentWindow.document
    } catch (e) {
        console.error("Cannot access iframe content due to cross-origin restrictions", e)
        return
    }
    Missing Null Check

    The code assumes the submit button always exists. Add a null check before calling click() to avoid runtime errors.

    const button = iframeDoc.getElementById('submit')
    button.click()
    console.log("Clicked submit button")
    Race Conditions

    Using fixed timeouts for loading screenly.js and the iframe can lead to unpredictable behavior. Consider event-based or promise-based loading.

    setTimeout(function() {
        // Get the dashboard iframe
        const dashboard = document.getElementById('dashboard')
    
        // Construct the URL after screenly.js has loaded
        let dashboardUrl
        try {
            // Try to use Screenly's CORS proxy
            dashboardUrl = screenly.cors_proxy_url + encodeURIComponent(dashboardUrlRaw)
        } catch (e) {
            // Fallback to direct URL if screenly is not available
            console.error("Error accessing screenly.cors_proxy_url, using direct URL", e)
            dashboardUrl = dashboardUrlRaw;
        }
    
        // Set dashboard URL
        dashboard.src = dashboardUrl
    
        // Attempt to inject passcode after iframe loads
        dashboard.onload = function() {
            setTimeout(injectPasscodeIntoIframe, 1000)
        };
    }, 500) // 500ms delay for screenly.js to load

    @github-actions
    Copy link

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Validate dashboard URL

    Use an empty default for the dashboard URL and bail out early if it’s not provided,
    avoiding loading an invalid placeholder string into the iframe.

    edge-apps/klipfolio-power-metrics-dashboard/script.js [4]

    -const dashboardUrlRaw = screenly.settings.dashboard_url || 'Dashboard URL not set'
    +const dashboardUrlRaw = screenly.settings.dashboard_url;
    +if (!dashboardUrlRaw) {
    +    console.error("Dashboard URL not set");
    +    return;
    +}
    Suggestion importance[1-10]: 6

    __

    Why: Bailing early on a missing URL prevents loading an invalid placeholder string into the iframe, improving robustness.

    Low
    Check iframe existence

    Add a guard after retrieving the dashboard iframe to ensure the element exists
    before setting properties on it. This prevents null reference errors if the iframe
    ID is incorrect or missing.

    edge-apps/klipfolio-power-metrics-dashboard/script.js [12]

    -const dashboard = document.getElementById('dashboard')
    +const dashboard = document.getElementById('dashboard');
    +if (!dashboard) {
    +    console.error("Dashboard iframe element not found");
    +    return;
    +}
    Suggestion importance[1-10]: 5

    __

    Why: The guard ensures dashboard is non-null before using it, preventing potential runtime errors when setting properties.

    Low
    Verify submit button exists

    Add a check to verify the submit button exists before calling .click() to prevent
    runtime errors if the button ID changes or is missing.

    edge-apps/klipfolio-power-metrics-dashboard/script.js [59-60]

    -const button = iframeDoc.getElementById('submit')
    -button.click()
    +const button = iframeDoc.getElementById('submit');
    +if (button) {
    +    button.click();
    +    console.log("Clicked submit button");
    +} else {
    +    console.error("Submit button not found");
    +}
    Suggestion importance[1-10]: 5

    __

    Why: Checking for the submit button before calling .click() prevents errors if the button is missing or renamed.

    Low
    General
    Use load event listener

    Replace the assignment to onload with an event listener to avoid overwriting other
    load handlers and ensure better event management.

    edge-apps/klipfolio-power-metrics-dashboard/script.js [29-31]

    -dashboard.onload = function() {
    -    setTimeout(injectPasscodeIntoIframe, 1000)
    -};
    +dashboard.addEventListener('load', () => {
    +    setTimeout(injectPasscodeIntoIframe, 1000);
    +});
    Suggestion importance[1-10]: 4

    __

    Why: Using addEventListener avoids overwriting other load handlers and is a more flexible event management pattern.

    Low

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Projects

    None yet

    Development

    Successfully merging this pull request may close these issues.

    1 participant