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.

Sign up for GitHub

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

Jump to bottom

[ClockFace] Fix setUI/Bangle.loadWidgets order, new option "loadWidgets" #1867

Closed
wants to merge 2 commits into from
Closed

[ClockFace] Fix setUI/Bangle.loadWidgets order, new option "loadWidgets" #1867

wants to merge 2 commits into from

Conversation

alessandrococco
Copy link
Contributor

@alessandrococco alessandrococco commented May 22, 2022 •
edited

I moved the Bangle.loadWidgets call after the setUI so CLOCK gets defined (so widclose won't be shown in the clock). I also add a new option "loadWidgets" (default true) so a developer can easily enable/disable the widgets in their clock.

@rigrig I took the liberty of making a couple of changes, hopefully that's not a problem! I'm working on a custom clock and your ClockFace is really cool, thanks!

@alessandrococco alessandrococco changed the title [Clock Face] [Clock Face] Fix setUI/Bangle.loadWidgets order, new option "loadWidgets" May 22, 2022
@alessandrococco alessandrococco mentioned this pull request May 22, 2022
Merged
@alessandrococco alessandrococco changed the title [Clock Face] Fix setUI/Bangle.loadWidgets order, new option "loadWidgets" [ClockFace] Fix setUI/Bangle.loadWidgets order, new option "loadWidgets" May 22, 2022
@rigrig
Copy link
Contributor

rigrig commented May 22, 2022

I'm always happy to see my code improved, however I have some concerns about this PR:

As I mention in this PR, moving loadWidgets after init will mess up Bangle.appRect, meaning all clocks that use the Layout library.

But more importantly, I don't think we want clocks to disable the widgets, I think it should be a global setting. That way clocks can use Bangle.appRect to work either way, and users can decide if they want widgets independently from which clock they install.

@alessandrococco
Copy link
Contributor Author

As I mention in this PR, moving loadWidgets after init will mess up Bangle.appRect, meaning all clocks that use the Layout library.

This could explain the weird issues I'm encountering (it's my first time with both ClockFace and Layout). Good to know, thank you!

I don't think we want clocks to disable the widgets, I think it should be a global setting.

A global setting (+ #1466) would be awesome!

Thank you for your comment

I close this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@alessandrococco @rigrig