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

Calendar Module QOL Features and Adjustments #3033

Merged
merged 14 commits into from
Feb 15, 2023

Conversation

CarJem
Copy link
Contributor

@CarJem CarJem commented Feb 11, 2023

This commit adds several QOL features and adjustments to the calendar module including:

  • New Options
    • coloredText: (default: false) Determines if you want your entry text to be colored based on the calendar's color
    • coloredBorder: (default: false) Determines if you want entry borders to be colored based on the calendar's color
    • coloredSymbol: (default: false) Determines if you want entry symbols to be colored based on the calendar's color
    • coloredBackground: (default: false) Determines if you want entry backgrounds to be colored based on the calendar's color

      These new colored options allows for more out-of-box styling options for the calendar module. With this the coloredSymbolOnly option has been removed due to redundancy

    • limitDaysNeverSkip: (default: false) show every event for every day regardless of if the day only has a single full day event
    • flipDateHeaderTitle: (default: false) determines if the title for the date header in the dateheaders time format should align to the left [eg: false] or right [eg: true]
  • Layout Changes
    • dateheader is now a class avaliable for date headers in the dateheaders time format.
    • Event entries have been better container-ized for better styling (using the event-container class)
    • repeatingCountTitle now has a seperator between the yearDiff and repeatingCountTitle
    • endDate in dateheaders now capitalizes it's first letter

modules/default/calendar/calendar.css Outdated Show resolved Hide resolved
modules/default/calendar/calendar.js Show resolved Hide resolved
modules/default/calendar/calendar.js Outdated Show resolved Hide resolved
@CarJem CarJem requested a review from rejas February 12, 2023 18:03
@CarJem
Copy link
Contributor Author

CarJem commented Feb 12, 2023

I have addressed all of the issues, minus the style issue, which I believe is necessary to ensure that most layouts don't break under these changes.

modules/default/calendar/calendar.js Show resolved Hide resolved
modules/default/calendar/calendar.js Outdated Show resolved Hide resolved
@CarJem
Copy link
Contributor Author

CarJem commented Feb 15, 2023

P.S. If you would like to speed this up I can be reached via Discord.

@CarJem CarJem requested a review from rejas February 15, 2023 17:12
Copy link
Collaborator

@rejas rejas left a comment

Choose a reason for hiding this comment

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

just one small thing...

Copy link
Collaborator

@rejas rejas left a comment

Choose a reason for hiding this comment

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

like we say in german: "zu früh gefreut". found a bug ....

@CarJem
Copy link
Contributor Author

CarJem commented Feb 15, 2023

Okay, how do I resolve these checks?

@CarJem
Copy link
Contributor Author

CarJem commented Feb 15, 2023

I guess I'll start by running prettier again

@rejas
Copy link
Collaborator

rejas commented Feb 15, 2023

Okay, how do I resolve these checks?

Do
npm run lint:prettier
and commit the changes to fix the tests.

@CarJem CarJem requested a review from rejas February 15, 2023 17:22
Copy link
Contributor Author

@CarJem CarJem left a comment

Choose a reason for hiding this comment

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

Done

modules/default/calendar/calendar.js Outdated Show resolved Hide resolved
modules/default/calendar/calendar.js Show resolved Hide resolved
modules/default/calendar/calendar.js Outdated Show resolved Hide resolved
@rejas
Copy link
Collaborator

rejas commented Feb 15, 2023

Grmpf, some other changes (not the css ones) were crashing my layout. Will look into it...

@CarJem
Copy link
Contributor Author

CarJem commented Feb 15, 2023

I was gonna put something in the changelog... but I don't know how to summarize this nicely

@CarJem
Copy link
Contributor Author

CarJem commented Feb 15, 2023

Grmpf, some other changes (not the css ones) were crashing my layout. Will look into it...

Okay, currently working on not alot of time... Once the hour is up, I must return to my day job... hopefully we can still make traction

@rejas
Copy link
Collaborator

rejas commented Feb 15, 2023

Dinner time here... Seems the newly added event-tables are the culprit...

@CarJem
Copy link
Contributor Author

CarJem commented Feb 15, 2023

Dinner time here... Seems the newly added event-tables are the culprit...

I'd like to refer to them as eventContainers please, anyways, I apologize, I had to merge this from an old build into the present and it hasn't been easy to do as you can tell...

@CarJem
Copy link
Contributor Author

CarJem commented Feb 15, 2023

Dinner time here... Seems the newly added event-tables are the culprit...

If you can provide me some logs I can diagnose

@rejas
Copy link
Collaborator

rejas commented Feb 15, 2023

You should get rid of the eventContainer and add your new styles to the eventWrapper. Then it looks like before. Would that break your personal layout?

@CarJem
Copy link
Contributor Author

CarJem commented Feb 15, 2023

You should get rid of the eventContainer and add your new styles to the eventWrapper. Then it looks like before. Would that break your personal layout?

Yes, and along with the new control for styling too

@rejas
Copy link
Collaborator

rejas commented Feb 15, 2023

I was gonna put something in the changelog... but I don't know how to summarize this nicely

"Added new calendar options for colored entries: ....." sounds nice :-)

@rejas
Copy link
Collaborator

rejas commented Feb 15, 2023

Yes, and along with the new control for styling too

But the existing layout depend on the table strcuture so that icons / title /time are aligned correctly. Your new eventContainer div breaks up the table and therefore breaks the exisiting layouts. Now off to dinner, sorry

@CarJem
Copy link
Contributor Author

CarJem commented Feb 15, 2023

Yes, and along with the new control for styling too

But the existing layout depend on the table strcuture so that icons / title /time are aligned correctly. Your new eventContainer div breaks up the table and therefore breaks the exisiting layouts. Now off to dinner, sorry

I'll see what can be changed then.... ugh....

@CarJem
Copy link
Contributor Author

CarJem commented Feb 15, 2023

Surprisingly, this should make both legacy and new layouts work with these changes. The only questionable change that is required for this to work was moving the location into the eventWrapper.

@CarJem
Copy link
Contributor Author

CarJem commented Feb 15, 2023

Surprisingly, this should make both legacy and new layouts work with these changes. The only questionable change that is required for this to work was moving the location into the eventWrapper.

Of which I just reverted and properly implemented

@rejas rejas merged commit e24dfa6 into MagicMirrorOrg:develop Feb 15, 2023
@rejas
Copy link
Collaborator

rejas commented Feb 15, 2023

Merging it finally, thanks for your patience.

Would be cool if you could prepare a PR for the documentation (https://github.com/MichMich/MagicMirror-Documentation) with the new config options.

Aaaand if you really got time (soo optional), maybe write tests for the new options? ;-)

@CarJem
Copy link
Contributor Author

CarJem commented Feb 15, 2023

Merging it finally, thanks for your patience.

Would be cool if you could prepare a PR for the documentation (https://github.com/MichMich/MagicMirror-Documentation) with the new config options.

Aaaand if you really got time (soo optional), maybe write tests for the new options? ;-)

I'll try to accomplish that this weekend

CarJem added a commit to CarJem/MagicMirror-Documentation that referenced this pull request Feb 19, 2023
rejas pushed a commit to MagicMirrorOrg/MagicMirror-Documentation that referenced this pull request Apr 5, 2023
* Changes for MagicMirrorOrg/MagicMirror#3033 and #151

* Fix linting

* Fix linting

---------

Co-authored-by: veeck <[email protected]>
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