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

Preview most recent version of images #53

Merged
merged 3 commits into from
Aug 6, 2015
Merged

Conversation

Galadirith
Copy link
Collaborator

This is a WIP towards #49

@Galadirith
Copy link
Collaborator Author

Still needs specs and currently doesn't condition on local or remote images (and I don't know how pathwatcher responds to remote images)

@leipert
Copy link
Contributor

leipert commented Aug 5, 2015

what is the refresh condition? pathwatch tells you that the file changed?

# Use most recent version of image
if imgVersion[src]?
if imgVersion[src] > 0
src = "#{src}##{imgVersion[src]}"
Copy link
Contributor

Choose a reason for hiding this comment

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

I would like "#{src}?v=#{imgVersion[src]}" more 👍

@Galadirith
Copy link
Collaborator Author

what is the refresh condition? pathwatch tells you that the file changed?

Exactly that. Pathwatcher will watch for any changes on the file, and then increment the imgVersion array for src and then triggers all open previews to re-render.

The debounce is to cover multiple fires as described in atom/node-pathwatcher#50.

Also if an image was previewed and subsequently deleted the version
query takes the value `deleted`.
@Galadirith
Copy link
Collaborator Author

All specs are passing, and I think the specs I added for this PR give good coverage. I'll review tomorrow with fresher eye's and if everything's still looking good I'll merge in to master :D

@leipert
Copy link
Contributor

leipert commented Aug 6, 2015

Looks really nice, especially your test png 💃 👍

@Galadirith
Copy link
Collaborator Author

@leipert Thanks for the eye's on :D

@Galadirith Galadirith merged commit 831c903 into master Aug 6, 2015
Galadirith added a commit that referenced this pull request Aug 6, 2015
@Galadirith Galadirith deleted the render-modified-images branch August 6, 2015 12:59
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