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.

Sign up for GitHub

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

Jump to bottom

Feature/thumbnails #128

Closed
wants to merge 4 commits into from
Closed

Feature/thumbnails #128

wants to merge 4 commits into from

Conversation

mro
Copy link

@mro mro commented Feb 26, 2015

see discussion at #111

@mro
#125 thumbnails: enable client-side caching
b30a147
@mro
#126 thumbnails: cleanup after failed resize.
ceeb91e
@mro
#111 thumbnails:
d1fa472 
- use meta/@og:image and link/@image_src,
- purge obsolete special treatment for ted.com, flickr.com, vimeo.com,
- code clean, DRY
@nodiscc nodiscc mentioned this pull request Feb 26, 2015
Closed
@nodiscc
Copy link
Member

nodiscc commented Feb 26, 2015

Comments at #111 (comment)

@mro
#111 thumbnails: gently trade some site specific php code for site sp…
3d338d4 
…ecific regexps/configuration. A bit DRYer and cleaner.

TODO: test flickr image links!

see also #128
@nodiscc
Copy link
Member

nodiscc commented Mar 5, 2015

@mro I've just tested your branch and it breaks all thumbnailing on my Shaarli instance. I'd be ok to merge it (if it works ;) ) as it improves the thumbnailing system without adding much complexity.

Can you rebase on top of master and do some testing?

I'm trying to find what's broken. Nothing in my server's logs so far.

@nodiscc nodiscc mentioned this pull request Mar 5, 2015
Closed
@mro
Copy link
Author

mro commented Mar 5, 2015

I have to confirm that already current master (3a10fa0) doesn't load any thumbnails on the 'picture wall'. Even before merging the pull request (3d338d4).

The 'daily' screen however shows thumbnails fine - both before and after applying the pull request.

@nodiscc
Copy link
Member

nodiscc commented Mar 5, 2015

@mro I'm running the current master and the picture wall works fine for me (Firefox 35). Can you check your browser console and server error logs.

Merging this pull request breaks thumbnails on the main link list (did not test other pages).

@mro
Copy link
Author

mro commented Mar 5, 2015

#128 (comment) ic - my bad, I just updated index.php - not all files (including the client-side lazy load javascript).

@mro
Copy link
Author

mro commented Mar 5, 2015

sorry, can't reproduce the problem from #128 (comment). Don't want to debug code that was added days after this pull request. Feel free to reject the pull request. Take it or leave it.

@nodiscc nodiscc self-assigned this Mar 6, 2015
@nodiscc
Copy link
Member

nodiscc commented Mar 6, 2015

I'll try to find what's wrong and merge properly.

@nodiscc nodiscc mentioned this pull request Mar 7, 2015
Closed
@nodiscc
Copy link
Member

nodiscc commented Mar 11, 2015

Moved to #153

@nodiscc nodiscc closed this Mar 11, 2015
ArthurHoaro pushed a commit to ArthurHoaro/Shaarli that referenced this pull request Jun 25, 2015
@mro @ArthurHoaro
shaarli#111 thumbnails: gently trade some site specific php code for …
cf46791 
…site specific regexps/configuration. A bit DRYer and cleaner.

TODO: test flickr image links!

see also shaarli#128
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@nodiscc nodiscc

Labels
enhancement in progress please test
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@mro @nodiscc