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

Enhancement: multiple widgets per service #4338

Merged
merged 6 commits into from
Nov 27, 2024

Conversation

DamitusThyYeetus123
Copy link
Contributor

@DamitusThyYeetus123 DamitusThyYeetus123 commented Nov 27, 2024

Proposed change

Allows for having multiple widgets per service. Syntax is as follows:

- Emby:
    icon: emby.png
    href: http://emby.host.or.ip/
    description: Movies & TV Shows
    widgets:
      -  type: emby
          url: http://emby.host.or.ip
          key: apikeyapikeyapikeyapikeyapikey
      -  type: uptimekuma
          url: http://uptimekuma.host.or.ip:port
          slug: statuspageslug

Closes #3024

Type of change

  • New service widget
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Documentation only
  • Other (please explain)

Checklist:

  • If applicable, I have added corresponding documentation changes.
  • If applicable, I have reviewed the feature and / or service widget guidelines.
  • I have checked that all code style checks pass using pre-commit hooks and linting checks.
  • If applicable, I have tested my code for new features & regressions on both mobile & desktop devices, using the latest version of major browsers.

@shamoon shamoon changed the title Features: Add multiple widgets per service Enhancement: multiple widgets per service Nov 27, 2024
@DamitusThyYeetus123
Copy link
Contributor Author

This should not be merged right now due to a security issue, will fix in a few minutes

@DamitusThyYeetus123
Copy link
Contributor Author

That commit should fix it up now

Copy link
Collaborator

@shamoon shamoon left a comment

Choose a reason for hiding this comment

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

Im not sure how much testing you've done here, but this seems to break many widgets out of the box. I'm not sure why yet

e.g. using the exact same widget configs

     - Multiple:
          widgets:
               - One:
                    type: unifi
                    ...
               - Two:
                    type: homebridge
                    ...
     - Unifi:
          description: Unifi Console Management
          widget:
               type: unifi
               ...
     - Homebridge:
          description: HomeKit for the impatient
          widget:
               type: homebridge
               ...
Screenshot 2024-11-27 at 12 31 00 AM

@DamitusThyYeetus123
Copy link
Contributor Author

I think I can see the issue, likely to do with how the proxying system works with having multiple widgets. Will check that now.

@DamitusThyYeetus123
Copy link
Contributor Author

Yup, those both have their own proxy handler which pulls the widget using getServiceWidget(), which I had to make changes to in order to allow pulling multiple widgets from the same service.

@DamitusThyYeetus123
Copy link
Contributor Author

Updated the fork, should fix those bugs with multiple widgets and some widgets not working

@shamoon
Copy link
Collaborator

shamoon commented Nov 27, 2024

Please see my changes:

  • Use index instead of name, since the name is pointless
  • A lot of code cleanup
  • Linting

Let me know if you note anything, probably should do more testing

@DamitusThyYeetus123
Copy link
Contributor Author

Yup, had a look at the changes. All seems good to me. In terms of testing, I do not have access to all of the services to test them individually, but I do agree that it should be put through more testing before merging. Not too sure how that testing should be done though.

@shamoon shamoon enabled auto-merge (squash) November 27, 2024 10:23
@shamoon shamoon merged commit da17f1b into gethomepage:dev Nov 27, 2024
3 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.

2 participants