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

[AssetMapper] bootstrap icons works with direct link not with assetmapper require #52620

Closed
tacman opened this issue Nov 16, 2023 · 3 comments
Closed

Comments

@tacman
Copy link
Contributor

tacman commented Nov 16, 2023

Symfony version(s) affected

6.4

Description

<link rel="stylesheet"
          href="https://cdn.jsdelivr.net/npm/bootstrap-icons@1.11.1/font/bootstrap-icons.min.css">

works as expected, but

bin/console importmap:require bootstrap-icons/font/bootstrap-icons.min.css

doesn't.

See https://bootstrap-icons-bug.survos.com/ for a working and non-working version.

How to reproduce

symfony new bootstrap-icons-bug --webapp --version=next --php=8.2 && cd bootstrap-icons-bug
composer config minimum-stability beta
composer config extra.symfony.allow-contrib true
composer req symfony/asset-mapper:^6.4 
bin/console importmap:require bootstrap-icons/font/bootstrap-icons.min.css
echo "import 'bootstrap-icons/font/bootstrap-icons.min.css'" >> assets/app.js


bin/console make:controller App -i
sed -i "s|/app|/|" src/Controller/AppController.php 

cat > templates/app.html.twig <<'END'
{% extends 'base.html.twig' %}
{% set useAssetMap = app.request.get('useAssetMap', false) %}
{% block javascripts %}
    {{ useAssetMap ? parent() }}
{% endblock %}

{% block stylesheets %}
    {% if not useAssetMap %} <link rel="stylesheet"
          href="https://cdn.jsdelivr.net/npm/bootstrap-icons@1.11.1/font/bootstrap-icons.min.css">
    {% endif %}
{% endblock %}

{% block body %}
    {% for useAssetMap in [true, false] %}
    <p>
        <a href="{{ path('app_app', {useAssetMap:useAssetMap}) }}">{{ useAssetMap ? 'with downloaded assetmap file' : 'with link tag to stylesheet on jsdelivr' }} </a>
        </p>
    {% endfor %}
    
    icon: <span class="bi bi-github"></span>
{% endblock %}

END

symfony server:start -d
symfony open:local

Possible Solution

There's something amiss with parsing out the fonts, the javascript console gives some warnings/errors.

Additional Context

No response

@smnandre
Copy link
Contributor

This package is not delivered as en ESM bundle, and the package.json does not list the woff / woff2 files so it may be something to digg

As stated in the documentation, when you need to deal with non ESM packages, sometimes it just works... and sometimes you have a bit of manual work to do :)

@tacman
Copy link
Contributor Author

tacman commented Nov 17, 2023

@smnandre yes, but @weaverryan has done a lot of work to handle more cases in 6.4. Given the ubiquity of bootstrap-icons, if it doesn't work out of the box, perhaps he could give guidance to the developer as to what's needed so that it does. There were issues with datatables.net that were resolved when the two of them looked at the problem together.

@fabpot fabpot closed this as completed Nov 25, 2023
fabpot added a commit that referenced this issue Nov 25, 2023
… in CSS (weaverryan)

This PR was squashed before being merged into the 6.4 branch.

Discussion
----------

[AssetMapper] Fix: also download files referenced by url() in CSS

| Q             | A
| ------------- | ---
| Branch?       | 6.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Issues        | Fix #52620
| License       | MIT

Hi!

`@tacman` found another situation where our "asset downloader" wasn't complete.

If we're downloading a CSS file, it may reference other files via `url()`. In #52620, these were font files, but they could be images or even other CSS Files. Currently, we do NOT download these, so the local references fail. This fixes that. I tried to keep the PR as small as possible, given the late stage of 6.4. But this *is* a bug fix: the downloader currently doesn't work for CSS files with `url()` inside.

Tested locally on ux.symfony.com with the bootstrap-icons package.

Cheers!

Commits
-------

ca90ed8 [AssetMapper] Fix: also download files referenced by url() in CSS
@tacman
Copy link
Contributor Author

tacman commented Nov 26, 2023

I can confirm that the above script now works with the -dev branch of 6.4. THANKS!! I'm guessing we'll see RC2 very soon.

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