/preview

Please find a preview at: https://staging.nodejs.dev/1713/

Alright, im pretty confused. I have no idea why its failing here, but everything passes locally. I tried creating a new branch bc I thought that might be it, commenting out the cache step in the test workflow, and even nuking node modules locally and installing with npm ci. Does anyone else know why its failing?

After digging around, I think its failing because the commit I pushed before was cached so now itll keep failing even though I uninstalled @emotion/react. Does anyone know how to bust the cache? I found this, maybe we can create a secret for it to bust the cache? For now im gonna manually bust it

Guess not, alright im officially out of ideas

I could reproduce same issue with build locally so it's not related to CI cache.

I assume this is due to @emotion/react is actually a peerDependency of react-google-calendar

Locally you are probably using Current node version with npm 7 which installs peer deps. But CI uses node v14 and does not do this by default.

It looks like we need to add @emotion/react as dependency anyway :(

Ok ill add it back in and push it up. I thought it was weird I kept seeing emotion in my peer deps. Howd you know the npm version wasnt 7 in gh actions? I assumed if it was the same version I had locally it would be fine

Howd you know the npm version wasnt 7 in gh actions?

There is an info step - https://github.com/nodejs/nodejs.dev/runs/3312719284?check_suite_focus=true#step:4:4

Codecov Report

Merging #1713 (6475d1a) into main (41c7636) will decrease coverage by 11.86%.
The diff coverage is 15.43%.

@@             Coverage Diff             @@
##             main    #1713       +/-   ##
===========================================
- Coverage   90.77%   78.91%   -11.87%     
===========================================
  Files          80       83        +3     
  Lines         867     1029      +162     
  Branches      239      284       +45     
===========================================
+ Hits          787      812       +25     
- Misses         79      216      +137     
  Partials        1        1               

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 41c7636...6475d1a. Read the comment docs.

/preview

Please find a preview at: https://staging.nodejs.dev/1713/

There are still some minor contrast issues in a dark mode that might be a blocker

I agree with the nitpicks you mentioned @joesepi and @manishprivet . I was thinking that work could be done in a separate PR though. I imagined this would be a "phase 1" of sorts, and we could iterate upon it. Especially considering how big this Pr is getting.

I agree with the nitpicks you mentioned @joesepi and @manishprivet . I was thinking that work could be done in a separate PR though. I imagined this would be a "phase 1" of sorts, and we could iterate upon it. Especially considering how big this Pr is getting.

Normally, I prefer smaller PRs but this is a big endeavor. I think that if you can address these remaining nits, it would be good to land them in this PR, in my opinion. Thanks!

@joesepi @manishprivet I made most of the requested changes detailed below:

• Contrast issue addressed by using color scheme the side nav uses in the learn section
• Increased font size for events on month view
• Increased calendar "title" font size and weight
• Added padding to events in month view so they arent so close to the edges
• (Side Note): Removed hardcoded values from calendar.scss and replaced with variables from tokens.scss

The only thing I wasnt able to add was making the time bold in the event label for the month view. The reason is because the titleAccessor function react-big-calendar accepts wants a string returned, not html. We can provide a custom component to the event as described here but I really think that should be a separate ticket.

Please find a preview at: https://staging.nodejs.dev/1713/

@lancemccluskey The contrast issue still doesn't seem to be fixed here

I was talking about the selected entry in calendar

Please find a preview at: https://staging.nodejs.dev/1713/

@manishprivet What about this? I messed around with the background, but it was hard to find a brand color that looked good on dark and light mode so I went with just changing the font color. I think its more pleasing to the eye and easier to read than before.

That looks much better

This looks a lot better @lancemccluskey

Can we do something similar here?

Please find a preview at: https://staging.nodejs.dev/1713/

I'm closing this PR as it is stale, and this PR is actively being worked on.