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

example.html doesn't replace the whole holder.js line #27395

Closed
XhmikosR opened this issue Oct 11, 2018 · 13 comments
Closed

example.html doesn't replace the whole holder.js line #27395

XhmikosR opened this issue Oct 11, 2018 · 13 comments

Comments

@XhmikosR
Copy link
Member

This

{{- include.content | replace: 'data-src="holder.js', 'src="...' -}}

Matches only the holder.js part and not the whole string. As far as I can tell there's no regex support here, so I'm not sure how to proceed.

/CC @m5o

@XhmikosR XhmikosR added the docs label Oct 11, 2018
@Johann-S Johann-S added the v4 label Oct 11, 2018
@m5o
Copy link
Contributor

m5o commented Oct 11, 2018

This was the idea, as far as I remember. When you look at the output of the markup example, the data-src="holder.js is replaced by src="... to not run holder.js twice.

<div class="card" style="width: 18rem;">
  <img class="card-img-top" src=".../100px180/" alt="Card image cap">
  <div class="card-body">
    <h5 class="card-title">Card title</h5>
    <p class="card-text">Some quick example text to build on the card title and make up the bulk of the card's content.</p>
    <a href="#" class="btn btn-primary">Go somewhere</a>
  </div>
</div>

📝 https://getbootstrap.com/docs/4.1/components/card/#example

@XhmikosR
Copy link
Member Author

Nope, the idea was to replace the whole data-src=holder.js... with src=....

@morrissey-ingenious
Copy link
Contributor

morrissey-ingenious commented Oct 11, 2018

@m5o
Copy link
Contributor

m5o commented Oct 11, 2018

For reference, this #25784 (comment) is the last comment before @XhmikosR continued and did some final changes and merged the PR.


If regex dosn’t work out of the box and to make the example html working without a js dependency. What do you think to replace the holder.js prefix with //picsum.photos/? This would at least create a working image url.

@XhmikosR
Copy link
Member Author

I'd even go with a simple SVG too if we can make it work and just pass the fill color, width and height.

@morrissey-ingenious
Copy link
Contributor

@XhmikosR
Copy link
Member Author

Not sure what you mean. Embed doesn't have any placeholder images.

@morrissey-ingenious
Copy link
Contributor

@XhmikosR there is a sample YouTube video in the embed example that is not replaced by ... I was thinking that all src elements should be replaced with ... not just the placeholder data-src ones.

@XhmikosR
Copy link
Member Author

Well, ideally, we could do that. But it's only one case so it's not so important to just use highlight manually.

@XhmikosR
Copy link
Member Author

@m5o: so do you have any suggestions? Otherwise I guess we'll need to revert back to the plugin.

@m5o
Copy link
Contributor

m5o commented Oct 22, 2018

I'm traveling currently and don't have my tech stuff with me. Sorry, bad timing.

  • An inline SVG could work, but it won't be a short one-liner if I get the inline SVG correctly. IMHO this creates to much noise into the example markup.
  • Probably easiest would be a link to a spacer gif, self hosted (bootstrapcdn?) or hot linked (imgplaceholder.com / dummyimage.com / unsplash.com) which use the same url structure.

@XhmikosR
Copy link
Member Author

We can wait a bit more, no hurry :)

I already have #27491 open and ready to be merged and to be honest I feel like it's the best solution due to Liquid's lack of regular expression support.

But if we can come up with a simple patch with SVGs, we can decide then which one to merge.

@XhmikosR
Copy link
Member Author

XhmikosR commented Nov 2, 2018

@MartijnCuppens: any luck with the placeholder SVG solution?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants