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

New version of picturefill.js.wp exists #249

Closed
benlk opened this issue May 20, 2014 · 14 comments
Closed

New version of picturefill.js.wp exists #249

benlk opened this issue May 20, 2014 · 14 comments
Assignees
Labels
priority: high Either blocks work on a priority-normal task or a solution here informs other work. type: improvement

Comments

@benlk
Copy link
Collaborator

benlk commented May 20, 2014

The version included in Largo is 1.1.3; the current stable is 1.3.2.

@aschweigert
Copy link

Yeah, we should take a look at this. Last time we tried implementing picturefill.js is was still causing issues, might be better now.

@rnagle rnagle added this to the 0.4 milestone Jul 10, 2014
@aschweigert aschweigert modified the milestones: 0.5, 0.4 Nov 12, 2014
@aschweigert aschweigert added type: improvement priority: normal Must be completed before release of this version of plugin. labels Nov 12, 2014
@benlk
Copy link
Collaborator Author

benlk commented Dec 12, 2014

It's up to 2.1 now, but the 1.3 branch is still maintained. The differences are summarized here and boil down to:

  • 2.x has native support for <picture>,
  • 1.x means we don't have to change markup
  • 1.x is lighter-weight
  • 1.x falls back to an image when JavaScript is disallowed, where 2.x falls back to the alt text with no image

@benlk
Copy link
Collaborator Author

benlk commented Jan 5, 2015

There are two ways to update the Picturefill.js.w in Largo:

Using a submodule would make it easier for us to keep up to date with changes upstream, but installation of largo from git clone would require a post-clone git submodule init and git submodule update, or using git clone --recursive instead, which could be annoying for anyone using our code.

@rnagle @aschweigert What do you think is the better option?

@aschweigert
Copy link

I would lean more towards the submodule approach I think but right now picturefill isn't even being used I don't think because it was causing too many problems. So, before we update, we should make sure that that's the approach we want to use for responsive images, and if so, then we can update it and make sure our implementation is working as it needs to.

@benlk
Copy link
Collaborator Author

benlk commented Jan 5, 2015

What sort of problems was it causing?

@aschweigert
Copy link

Mostly issues dealing with different possible handlings of image/caption shortcodes. If you were starting off with a new site from scratch it seems like it was mostly fine but with migrations we were running into some weird problems so we deactivated it until we could thoroughly test it and make sure it was really 100%

@benlk
Copy link
Collaborator Author

benlk commented Jan 6, 2015

Submodules will make our lives significantly more annoying in the short term. Check out the picturefill1.3-submodule branch of Largo. It may give you warnings that you'll have to override with git checkout -f picturefill1.3-submodule. Now try to git checkout develop: you'll have to -f again. When you next checkout the picturefill1.3-submodule branch, the files in the inc/picturefill directory are messed up and need to be reset by going into the submodule directory to run git checkout -f origin/1.3.x.

Of course, this ceases to be a problem once all the branches that people are using are updated with commits from the submodule branch, but until then it will be quite annoying.

It's possibly better to just update the files, as in the picturefill1.3-files branch

@benlk
Copy link
Collaborator Author

benlk commented Jan 9, 2015

One way to get around the file conflicts with submodules is to throw the submodule in a differently-named directory. Instead of inc/picturefill, perhaps inc/picturefillwp.

@drywall
Copy link

drywall commented Jan 9, 2015

I haven’t been following this thread closely so I’m not sure what the submodule issues are, but have you considered something like subtree as an alternative approach? http://blogs.atlassian.com/2013/05/alternatives-to-git-submodule-git-subtree/

On Fri, Jan 9, 2015 at 4:52 PM, Ben Keith notifications@github.com
wrote:

One way to get around the file conflicts with submodules is to throw the submodule in a differently-named directory. Instead of inc/picturefill, perhaps inc/picturefillwp.

Reply to this email directly or view it on GitHub:
#249 (comment)

@benlk
Copy link
Collaborator Author

benlk commented Jan 12, 2015

Subtree actually looks pretty easy. Check out the picturefill1.3-subtree branch.

@benlk
Copy link
Collaborator Author

benlk commented Jan 21, 2015

There's also the picturefill-update branch, from Cornershop.

@aschweigert
Copy link

@rnagle rnagle modified the milestones: 0.5.1, 0.5 Apr 14, 2015
@rnagle rnagle added priority: high Either blocks work on a priority-normal task or a solution here informs other work. and removed priority: normal Must be completed before release of this version of plugin. labels May 4, 2015
@benlk
Copy link
Collaborator Author

benlk commented Jun 2, 2015

Yet another update: https://css-tricks.com/please-update-picturefill/

@aschweigert aschweigert modified the milestones: Backlog, 0.5.1 Jul 1, 2015
@benlk
Copy link
Collaborator Author

benlk commented Nov 8, 2018

We removed Picturefill in #1049 on Jan 7, 2016, but there's still some code from it floating around.

benlk added a commit that referenced this issue Nov 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: high Either blocks work on a priority-normal task or a solution here informs other work. type: improvement
Projects
None yet
Development

No branches or pull requests

4 participants