Skip to content

Fix calendar.js missing default symbol prefix #3007

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
Merged
rejas merged 11 commits into from
Jan 16, 2023
Merged

Fix calendar.js missing default symbol prefix #3007

rejas merged 11 commits into from
Jan 16, 2023

Conversation

@DaveChild
Copy link
Contributor

@DaveChild DaveChild commented Jan 14, 2023

Symbols provided in customEvents don't get the "fas fa-fw fa-" prefix, which according to the docs they should. This fixes that.

Hello and thank you for wanting to contribute to the MagicMirror² project

Please make sure that you have followed these 4 rules before submitting your Pull Request:

  1. Base your pull requests against the develop branch.

  2. Include these infos in the description:

  • Does the pull request solve a related issue?
  • If so, can you reference the issue like this Fixes #<issue_number>?
  • What does the pull request accomplish? Use a list if needed.
  • If it includes major visual changes please add screenshots.

  1. Please run npm run lint:prettier before submitting so that
    style issues are fixed.

  2. Don't forget to add an entry about your changes to
    the CHANGELOG.md file.

Note: Sometimes the development moves very fast. It is highly
recommended that you update your branch of develop before creating a
pull request to send us your changes. This makes everyone's lives
easier (including yours) and helps us out on the development team.

Thanks again and have a nice day!

@DaveChild
Fix calendar.js missing default symbol prefix
d081ffa 
Symbols provided in customEvents don't get the "fas fa-fw fa-" prefix, which according to the docs they should. This fixes that.
@khassel
Copy link
Collaborator

khassel commented Jan 14, 2023

Thanks!

Related to this forum thread.

Can you please add an entry into CHANGELOG.md under "Fixed" (we collect all PR's there for next release info), otherwise the tests will fail ...

@DaveChild
Copy link
Contributor Author

DaveChild commented Jan 14, 2023

I think that's done it. Sorry, I'm a bit rushed today or I'd have read the docs before committing etc :)

@codecov-commenter
Copy link

codecov-commenter commented Jan 14, 2023 •
edited
Loading

Codecov Report

Merging #3007 (f3f0abe) into develop (c714399) will increase coverage by 0.07%.
The diff coverage is 0.00%.

@@             Coverage Diff             @@
##           develop    #3007      +/-   ##
===========================================
+ Coverage    21.76%   21.84%   +0.07%     
===========================================
  Files           52       52              
  Lines        11439    11441       +2     
===========================================
+ Hits          2490     2499       +9     
+ Misses        8949     8942       -7
Impacted Files Coverage Δ
modules/default/calendar/calendar.js 35.56% <0.00%> (-0.08%) ⬇️
modules/default/updatenotification/node_helper.js 90.27% <0.00%> (+12.50%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@rejas
Copy link
Collaborator

rejas commented Jan 15, 2023

Thx for the PR. Do you feel adventerous enough to write test for this too?

@DaveChild DaveChild marked this pull request as draft January 16, 2023 19:34
@DaveChild
Copy link
Contributor Author

DaveChild commented Jan 16, 2023

Bah humbug. Ok, I'll give it a try :)

@DaveChild
Copy link
Contributor Author

DaveChild commented Jan 16, 2023

Ok, I've had a crack at a test. Sorry, code's a bit of a mess, not used jest etc before, so took a while to get to grips with. I've added a new test to make sure the customEvent symbol is set, and it passes on my dev system.

@DaveChild DaveChild marked this pull request as ready for review January 16, 2023 20:22
@DaveChild DaveChild requested a review from khassel January 16, 2023 20:25
@rejas rejas merged commit 4244c05 into MagicMirrorOrg:develop Jan 16, 2023
@rejas
Copy link
Collaborator

rejas commented Jan 16, 2023

Thx for the PR and the tests! Good job!

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

Reviewers

@rejas rejas rejas approved these changes

@khassel khassel Awaiting requested review from khassel

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@DaveChild @khassel @codecov-commenter @rejas