Skip to content
This repository has been archived by the owner on Oct 23, 2023. It is now read-only.

fix: remove redundant event listeners #532

Merged
merged 1 commit into from
Jun 7, 2018

Conversation

barinali
Copy link
Contributor

@barinali barinali commented Jun 7, 2018

Hello,

Thanks for all the effort you are putting in Alva and the given lightning talk at JSConf EU 2018.

I have been looking at Alva's code and realised there are some redundant listeners. So I thought I could contribute to it a little bit.

There is no open issue about this. So I hope it's not a problem that I am just trying to fix a thing I have found on my own without creating an issue here.

The issue is; whenever a user closes and opens a window in Alva, listeners which are in src/electron/main.ts file were re-attached. Therefore, after closing & opening Alva window several times, the actions in listeners were executed as many as it's re-opened. Then for example, if a user tries to open an existing Alva file, (s)he sees multiple "open file" dialog.

A screenshot when an existing Alva file is opened after I re-opened Alva's window five times;
image

For the review; I would suggest reading the file. Because here, the change does not look like properly. What I have done is; I have taken the server creation and the listeners out of createWindow and put it in an IIFE (since that part was async and there is no top-level async support) in order to avoid the redundant load and fix the application behaviour for user experience.

@lkuechler
Copy link
Member

Hi @barinali,
Thank you very much for the kind words and especially for the pull request.
I just tested it and it works really good. I will merge it and mention you in the next release notes.

Also thanks for the great description on what you changed and why. This made it very easy to understand what was changed. :)

@lkuechler lkuechler merged commit 9c35fe5 into meetalva:master Jun 7, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants