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

Feature/Restore last opened files #159

Open
wants to merge 16 commits into
base: v1.x/staging
Choose a base branch
from

Conversation

miteshgoplani
Copy link

@miteshgoplani miteshgoplani commented Jul 18, 2020

This PR serves as a feature enhancement for Zlux Editor - Restore last openend files/datasets in Editor on next launch

  • Save file path on file/dataset open
  • Delete file path on file/dataset close
  • Restore last opened files/datasets on next launch

Dependent-on: zowe/zlux-app-manager#274

Signed-off-by: Mitesh Goplani [email protected]

@miteshgoplani miteshgoplani changed the title [WIP] Create open tabs saving mechanism Feature/Restore last opened files Jul 18, 2020
Signed-off-by: Mitesh <[email protected]>
Signed-off-by: Mitesh <[email protected]>
@1000TurquoisePogs
Copy link
Member

The concept works but there are some issues

  1. GET before PUT is not needed because the editor knows the list of every tab that is open, so you can just PUT. Because you already have the events in the right place, I suggest you can just have an array within the editor-control file that you can add to & remove from when you open and close tabs, and then do a PUT on the result of the array. This way, no GET is needed! It will improve performance and reduce cpu use.
  2. What to do when there are two editors open? Maybe saving should only work for the first one currently, because now it is weird that you get the same tabs twice.

@miteshgoplani
Copy link
Author

The concept works but there are some issues

  1. GET before PUT is not needed because the editor knows the list of every tab that is open, so you can just PUT. Because you already have the events in the right place, I suggest you can just have an array within the editor-control file that you can add to & remove from when you open and close tabs, and then do a PUT on the result of the array. This way, no GET is needed! It will improve performance and reduce cpu use.
  2. What to do when there are two editors open? Maybe saving should only work for the first one currently, because now it is weird that you get the same tabs twice.

Resolved the first issue
Working on the second one

@miteshgoplani
Copy link
Author

The concept works but there are some issues

  1. GET before PUT is not needed because the editor knows the list of every tab that is open, so you can just PUT. Because you already have the events in the right place, I suggest you can just have an array within the editor-control file that you can add to & remove from when you open and close tabs, and then do a PUT on the result of the array. This way, no GET is needed! It will improve performance and reduce cpu use.
  2. What to do when there are two editors open? Maybe saving should only work for the first one currently, because now it is weird that you get the same tabs twice.

For the second issue, added a window storage object activeZluxEditors which stores active editor windows count and if only one window is open fileRestore would work

webClient/src/app/app.component.ts Outdated Show resolved Hide resolved
webClient/src/app/app.component.ts Outdated Show resolved Hide resolved
Copy link
Member

@DivergentEuropeans DivergentEuropeans left a comment

Choose a reason for hiding this comment

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

Great work so far. For some reason,
Save file path on file/dataset open
Delete file path on file/dataset close

this seemed to work for 1 time, and then stopped working? I wonder if it's related to my comment

However, saved tabs do indeed open back up.

this.log.debug(`Monaco object=`,monaco);
let openWindowsStorageString : string = window.localStorage.getItem("org.zowe.editor-openWindows")
Copy link
Member

Choose a reason for hiding this comment

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

You don't have to hardcode the plugin name. You can get this from the PLUGIN_DEFINITION

Copy link
Member

Choose a reason for hiding this comment

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

this.pluginDefinition.getBasePlugin().getIdentifier() should work

Copy link
Author

Choose a reason for hiding this comment

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

Okay, I'll use this

dataToSave = activeZluxEditorsCount.toString() + ":" + Date.now()
window.localStorage.setItem("org.zowe.editor-openWindows",dataToSave)
window.addEventListener("beforeunload", () => {
activeZluxEditorsCount = +window.localStorage.getItem("org.zowe.editor-openWindows").split(":")[0]
Copy link
Member

Choose a reason for hiding this comment

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

Is this not a copy for what you do in NgOnDestroy? Don't they attempt to accomplish the same thing or do you need both?

Copy link
Member

Choose a reason for hiding this comment

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

Additionally, if you want to use the JS event listeners, you'll need to initialize them to a variable and manually removeEventListener() otherwise, they will continue existing until something else wipes them

Copy link
Author

Choose a reason for hiding this comment

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

Sometimes ngOnDestroy() for Editor is not called when we close the browser tab directly with the editor open.
So, to handle that case I was using JS event listener

@miteshgoplani
Copy link
Author

miteshgoplani commented Aug 26, 2020

Great work so far. For some reason,
Save file path on file/dataset open
Delete file path on file/dataset close

this seemed to work for 1 time, and then stopped working? I wonder if it's related to my comment

However, saved tabs do indeed open back up.

The editor would stop saving tabs when we have more than one instance of editor running in same/multiple windows
Was it the case when Save didn't occur?
@DivergentEuropeans

Copy link
Member

@DivergentEuropeans DivergentEuropeans left a comment

Choose a reason for hiding this comment

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

Changes look good to me 🍨 (code) just need to test/QA this. Please reach out to @struga0258 if I don't get to it

Copy link
Member

@1000TurquoisePogs 1000TurquoisePogs left a comment

Choose a reason for hiding this comment

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

This looks good to me now so we'll just merge when automation has been written which is something that James and I are looking into who will be doing that.

@DivergentEuropeans
Copy link
Member

Update: This PR is good, but doesn't address dealing with multiple Editor sessions and across multiple Desktops. It needs extending & may be groomed later by the team

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

3 participants