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

Adapting calendar to use a non-remote environment #1018

Merged
merged 3 commits into from
Nov 1, 2023

Conversation

araujoarthur0
Copy link
Collaborator

Related issue

Closes #889

Context / Background

Just like #887, this time for the calendar part.
"In #646 we are trying to update electron to the newest version, but we're blocked because we're using electron remote, and that is deprecated in the newest versions."

What change is being introduced by this PR?

In this patch I adapt the Calendar window to work through a preload script, and an isolated environment, without any node module dependencies outside of what the preload API offers.

Basically, I exposed an API on the new files renderer/preload-scripts/calendar-api.js and calendar-bridge.js using the ContextBridge electron object (see https://www.electronjs.org/docs/latest/api/context-bridge). This API is defined via the main process, and is set to the global window object, and can be accessed in the calendar.js file, loaded in a BrowserWindow, inside a new renderer process.

  • Moving a few files. index.js is removed and index.html is now calendar.html
  • Many functions had to be switched to the async model
  • Calendar constructor can no longer initialize the calendar store by itself since that requires an async operation. Had to add extra initialization steps to the factory
  • Many tests had to be changed.

How will this be tested?

Same as before. Manual tests also were executed.

@araujoarthur0 araujoarthur0 changed the title Adapting calendar Adapting calendar to use a non-remote environment Oct 11, 2023
@codecov
Copy link

codecov bot commented Oct 11, 2023

Codecov Report

Merging #1018 (cc32c2a) into main (e25523f) will increase coverage by 4.25%.
Report is 1 commits behind head on main.
The diff coverage is 20.00%.

@@            Coverage Diff             @@
##             main    #1018      +/-   ##
==========================================
+ Coverage   75.04%   79.30%   +4.25%     
==========================================
  Files          26       21       -5     
  Lines        2188     1261     -927     
  Branches      345      188     -157     
==========================================
- Hits         1642     1000     -642     
+ Misses        546      261     -285     
Files Coverage Δ
js/time-balance.js 100.00% <ø> (ø)
js/main-window.js 73.50% <87.50%> (+0.46%) ⬆️
js/menus.js 81.73% <50.00%> (ø)
src/calendar.js 0.00% <0.00%> (ø)

... and 4 files with indirect coverage changes

Copy link
Owner

@thamara thamara left a comment

Choose a reason for hiding this comment

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

Shiiiiiiiip it!

@araujoarthur0 araujoarthur0 merged commit df37f2c into thamara:main Nov 1, 2023
7 of 8 checks passed
tupaschoal added a commit to tupaschoal/time-to-leave that referenced this pull request Nov 1, 2023
araujoarthur0 pushed a commit that referenced this pull request Nov 1, 2023
* Remove unused variable from #1018

* Amend changelog to include fix from #1011
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.

Adapt Calendar window to an electron isolated environment
2 participants