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

[email protected] :Refactor Bing Wallpaper Applet Code for Enhanced Maintainability #6218

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

tesfayegirma-116
Copy link

Bing Wallpaper Cinnamon Applet

Overview

This repository contains the code for the Bing Wallpaper Cinnamon Applet. The applet sets the Bing daily wallpaper as your desktop background. Recent updates include:

  • Code Refactoring: Improved maintainability and organization.
  • Module Import Fixes: Resolved issues with module paths and imports.

Changes

Refactor and Fixes

  • Code Refactoring:

    • Separation of Concerns: Code split into distinct modules for better organization.
    • Enhanced Documentation: Added comprehensive comments and documentation for clarity.
  • Code Organization:

    • Best Practices: Followed best practices for file structure and module management.

File Structure

Ensure your file structure matches the following:

~/.local/share/cinnamon/applets/[email protected]/
├── applet.js
├── httpClient.js
├── fileManager.js
├── settingsManager.js
└── utils.js


@Starcross 

- Ensured correct file paths and naming conventions for JavaScript modules.
- Refactored code to improve maintainability and separation of concerns.
- Added detailed comments and documentation to clarify code functionality.
- Verified that all required modules are properly imported and utilized.

improves overall code structure.
@fredcw
Copy link
Contributor

fredcw commented Jul 19, 2024

Ping @Starcross

Copy link
Contributor

@Starcross Starcross left a comment

Choose a reason for hiding this comment

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

Thanks for your PR. I left a couple of comments, but I think basic functionality is also broken at the moment. Looking at the logs alone it seems like handling the metadata has an issue. I'm often seeing this error, so it's probably a good starting point:

info t=2024-07-20T18:05:11Z [[email protected]]: Unable to get local metadata: TypeError: this.imageData is undefined

* @returns {string} - URL of the image to download.
*/
getImageUrl: function () {
// Implementation depends on the structure of metadata.
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure I understand this comment. The intention here is to replace the resolution with _UHD, which will get the highest resolution image available

Copy link
Contributor

Choose a reason for hiding this comment

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

This image is almost certainly owned and copyright by Microsoft, and I would not expect them to permit this kind of use without permission.

I would argue this would also lend a bit of a microsoft/windows flavour to the applet, which I'm not sure our audience would enjoy

@rcalixte rcalixte marked this pull request as draft July 21, 2024 00:43
@tesfayegirma-116
Copy link
Author

Thanks for your PR. I left a couple of comments, but I think basic functionality is also broken at the moment. Looking at the logs alone it seems like handling the metadata has an issue. I'm often seeing this error, so it's probably a good starting point:

info t=2024-07-20T18:05:11Z [[email protected]]: Unable to get local metadata: TypeError: this.imageData is undefined

@Starcross No problem! Thanks for checking it out. It’s my first time asking for contributions, so I’m as excited as a puppy with two tails! 🐶✨

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.

3 participants