Navigation leverage icon core API

Created on 24 October 2024, about 2 months ago

Problem/Motivation

Follow up of Add an icon management API Active , use icon API for navigation module.

📌 Task
Status

Needs work

Version

11.0 🔥

Component

navigation.module

Created by

🇫🇷France mogtofu33

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @mogtofu33
  • Pipeline finished with Failed
    about 2 months ago
    Total: 183s
    #319647
  • Pipeline finished with Failed
    about 2 months ago
    Total: 834s
    #320913
  • 🇫🇷France pdureau Paris

    This MR is currently made of 2 parts:

    • Everything inside /core/asset , /core/lib and /core/tests is a duplicate of 📌 Navigation leverage icon core API Needs work which was taken as a starting point. OF course, this part will be removed when the other issue is merged into 11.x
    • the changes in /core/modules/navigation is the interesting part for this issue
  • First commit to issue fork.
  • 🇷🇸Serbia finnsky

    Thank you for great start. I see some bem mistakes. Gonna fix it.

  • 🇷🇸Serbia finnsky

    I don't think we should define viewbox somewhere

  • 🇫🇷France pdureau Paris

    I don't think we should define viewbox somewhere

    It depends of the icon pack we are implementing. Th goal of this Icon API is to provide simple tools to faithfully implement the icon packs as documented upstream by the icons provider.

    For example, Bootstrap Icons expects a viewBox, so we need to deal with it:

    <svg xmlns="http://www.w3.org/2000/svg" width="16" height="16" fill="currentColor" class="bi bi-emoji-heart-eyes" viewBox="0 0 16 16">
      <path d="M8 15A7 7 0 1 1 8 1a7 7 0 0 1 0 14zm0 1A8 8 0 1 0 8 0a8 8 0 0 0 0 16z"/>
      <path d="M11.315 10.014a.5.5 0 0 1 .548.736A4.498 4.498 0 0 1 7.965 13a4.498 4.498"/>
    </svg>
    

    In navigation module, we are both the icons provider and the icon pack definition author. So, we can do anything. If it works OK without the viewBox attribute, and you think it will be better without it, we can remove it.

  • 🇷🇸Serbia finnsky

    I think we should keep the original viewBox attribute if it is not reassigned by the template

    Here I added a simple illustration of how the absence of the original attribute breaks the adaptability of icons.

    https://codepen.io/finnsky/pen/jOgKdWz

  • 🇫🇷France mogtofu33

    I will add the viewbox available as a variable if set in the svg, this will match your needs and be an optional feature when people need it.

    I will update here when pushed on the main MR.

  • 🇷🇸Serbia finnsky

    @mogtofu33 thank you for this and for all that initiative

Production build 0.71.5 2024