Skip to content

Conversation

@abelgomez
Copy link
Contributor

Change Preview Viewer refresh strategy:

  • Preview Viewer uses an intermediate file, which enables the use of the refresh() method. The refresh()method does not reset the scroll position on every update of the viewer.
  • Preview viewer is refreshed automatically after a small delay while the user types in the Markdown editor (and not when the editor regains focus).
  • Old behaviour in which the preview viewer is refreshed when the View gains focus is maintained.

@paulvi
Copy link
Collaborator

paulvi commented Mar 10, 2016

It looks very great, I will merge, but giving some time to let Daniel @winterstein to take a look.

Old behaviour in which the preview viewer is refreshed when the View gains focus is maintained.

Do you mean "when View gets visible"? If it is not visible is should not be updated.

@abelgomez
Copy link
Contributor Author

I meant when the viewer gets the focus, since there's a call to update() in the setFocus() method of the MarkdownPreview ViewPart.

Anyway, I've seen that I forgot some checks about disposed widgets in my pull request, so my patch contains some bugs. You may want to close the request without merging, use it as a baseline, or wait for my modifications :)

@abelgomez abelgomez closed this Mar 10, 2016
@paulvi
Copy link
Collaborator

paulvi commented Mar 14, 2016

You could have leave it open add add more commits as well.

Have you checked issues here, I think it was discussed

@abelgomez
Copy link
Contributor Author

Hi @paulvi ,

to tell the truth, I didn't check the open issues, but this patch is a fix for #3.

In commit abelgomez@8e82af2 I fixed the most important issues, mostly the use of disposed widgets. The last week I've been using the patched version and everything seems to work fine.

However I detected that some things may be improved (enable/disable options for the feature, configurable delays, etc.). Additionally, the code may be improved (specially, I don't like the static field MarkdownPreview.preview for accessing the view).

Please, test the patch and see if you like the new behavior. Then we can decide if we implement the improvements.

Cheers!

@paulvi
Copy link
Collaborator

paulvi commented Mar 18, 2016

Well, frankly I will not have time for trying version from sources.
But I think for closer co-working open PR with note "work in progress" would be awesome.

Still hoping to hear from Daniel @winterstein

@paulvi paulvi reopened this Mar 18, 2016
@paulvi paulvi changed the title Keep position of the preview Viewer Keep position of the preview Viewer #3 work-in-progress Mar 18, 2016
@winterstein
Copy link
Owner

Hello @paulvi and @abelgomez,

This sounds good. Thank you. I'm happy for us to go ahead on Paul's say so. Sorry I wasn't able to join in here - I've been snowed under with other tasks.

Kind regards, Daniel

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.

3 participants