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

Noticeable delay each time in-product release notes editor is made active. #13257

Closed
gregvanl opened this issue Oct 5, 2016 · 7 comments
Closed
Labels
feature-request Request for new features or functionality good first issue Issues identified as good for first-time contributors help wanted Issues identified as good community contribution opportunities info-needed Issue requires more information from poster
Milestone

Comments

@gregvanl
Copy link

gregvanl commented Oct 5, 2016

  • VSCode Version:1.6.0 insiders
  • OS Version: Windows

Steps to Reproduce:

  1. Follow test plan to enable in product release notes in insiders Test: Release notes in the product #12520
  2. Help > Release Notes
  3. Note delay (~1.5 seconds) when page is initially displayed.
  4. Switch to a different editor and then go back to release notes.

The release notes editor is re-rendered and there is the same delay each time.
We should look into caching the rendered page since release notes are unlikely to change during the user's reading of the release notes. We don't need to cache release notes across sessions so the user will pick up any release note changes when they first open them (Help > Release Notes).

@gregvanl
Copy link
Author

gregvanl commented Oct 6, 2016

On my home wifi this can take up to 5 seconds (depending on what the kids are streaming).

@joaomoreno joaomoreno removed their assignment Oct 6, 2016
@joaomoreno joaomoreno added help wanted Issues identified as good community contribution opportunities info-needed Issue requires more information from poster good first issue Issues identified as good for first-time contributors labels Oct 6, 2016
@Tyriar Tyriar added the feature-request Request for new features or functionality label Oct 6, 2016
@olingern
Copy link
Contributor

I'm interested in helping out with this. A couple of questions for @joaomoreno @Tyriar

Idea:
Place a hook inside of loadReleaseNotes in src/vs/workbench/electron-browser/update.ts to see if a cached markup file exists, if it does load it instead of retrieving from https://code.visualstudio.com. Otherwise, load from url and write markup response to disk.

  1. Frequency of caching: Are release notes final, or do they often update without regards to a release?
  2. Location of file: I'm newer to the directory structure and thought out/vs/workbench/electron-browser/cache/release-notes.md may be a good place.

@joaomoreno joaomoreno added this to the Backlog milestone Oct 24, 2016
@joaomoreno
Copy link
Member

@olingern You shouldn't write them to disk, but just keep them in memory. It's OK if they take up time loading once Code restarts.

@olingern
Copy link
Contributor

@joaomoreno This one is a bit tricky as it doesn't seem I can test through the development build. Do you have any pointers for creating a build where I can see a full list of menu items?

screen shot 2016-10-27 at 8 42 40 pm

@joaomoreno
Copy link
Member

Yeah, you have to add to the top level product.json the following key:

    "releaseNotesUrl": "https://go.microsoft.com/fwlink/?LinkId=724002"

@olingern
Copy link
Contributor

olingern commented Nov 5, 2016

@joaomoreno Thanks!

I have a PR open, #15050, with a working fix, but a couple of issues:

  • linting
    The gulp hygiene task is telling me my file ( and all files ) aren't formatted. I saw a couple of other issues in the repo chatting about this. I installed typescript-formatter and ran tsfmt -r update.ts, but it produced an error

screen shot 2016-11-05 at 7 48 09 pm

Running gulp hygiene tells me all files in my project aren't formatted as well:

screen shot 2016-11-05 at 6 18 59 pm

- Tests

It wasn't clear to me where tests for this feature would live. Based upon where other tests live, it looks like src/vs/workbench/test/electron-browser/update.ts is where this should live, but the electron-browser test directory doesn't exist, at the moment.

@mjbvz
Copy link
Collaborator

mjbvz commented Apr 21, 2017

I believe this was fixed by #15050

@mjbvz mjbvz closed this as completed Apr 21, 2017
@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 18, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature-request Request for new features or functionality good first issue Issues identified as good for first-time contributors help wanted Issues identified as good community contribution opportunities info-needed Issue requires more information from poster
Projects
None yet
Development

No branches or pull requests

5 participants