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

Using fontawesome class in digital collection title breaks "SVG withJS" #169

Open
patdunlavey opened this issue Mar 18, 2022 · 6 comments
Open

Comments

@patdunlavey
Copy link
Contributor

If you go to font awesome's settings page and choose the recommended "SVG with JS" as the "Font Awesome Method", and view search results that includes a digital collection, rather than showing the folder icon, we seen an animated icon of a exclamation/question mark. This comes from the title field being configured in the "Digital Object with thumbnail and abstract" view mode with the classes "fas fa-archive" being applied to the h2 wrapper tag - which is not "legal" in font-awesome when using the "SVG with JS" method (it works fine using the to-be-deprecated "Web Fonts with CSS" method).

Here's where this is coded.

The correct markup should put these classes inside a <i> tag. I dealt with it in my local install by implementing hook_preprocess_field() to alter the markup. If we were ambitious, in a custom field widget/formatter might be the the more correct solution? Or possibly this module actually gives us the field formatter we need?

This issue is also present in archipelago_deployment_live. Should I make a separate issue there?

@patdunlavey
Copy link
Contributor Author

I'd be happy to work on this if/when you indicate a preference among the possible solutions, @DiegoPino.

@DiegoPino
Copy link
Member

DiegoPino commented Mar 18, 2022

@patdunlavey Ok, but we do not ship"SVG with JS" enabled nor a newer version of Font Awesome where CSS is deprecated. I feel this might be a thing if you are thinking of improving the Archipelago Sub theme as a module that pre processes data and that matches the (changed) settings here. But I'm not very eager to make changes of our reference configuration based on possible future changes people might do.

There is a more pressing need in the theming / render array interaction (going for a pull soon, reported by Jordan) where the the Field UI (e.g Changing a field formatter) with our current settings for Bootstrap (use input instead of buttons) break AJAX (Issue will come soon)

So, said differently, the reference deployment might document that if Something is changed then something might break? Or, if you are willing to go the full route, allow maybe even at the render array (themes can affect that) change something to make it work too with SVG with JS. But as it is now I see this as a lower priority. Please fight me back of course

@patdunlavey
Copy link
Contributor Author

Agreed, not a high priority, and documenting a solution, possibly just in this ticket (e.g. share my implementation of hook_preprocess_field), could be "good enough" for now. Here's that implementation:

/**
 * Implements hook_preprocess_HOOK() for field.html.twig.
 *
 * When a field title field has a fontawesome class, create
 * an <i> tag for hold the fontawesome glyph and prepend it
 * to the title.
 *
 */
function MY_THEME_preprocess_field(&$variables) {
  if($variables['field_name'] == 'node_title') {
    foreach($variables['items'] as &$item) {
      foreach($item['content'] as &$content) {
        if(!empty($content['#context']['attributes']) && !empty($content['#context']['wrapper'])) {
          /** @var Drupal\Core\Template\Attribute $attributes */
          $attributes = $content['#context']['attributes']->storage();
          if(!empty($attributes['class'])) {
            $class = $attributes['class']->__toString();
            if(strpos($class, 'fas') !== FALSE && $content['#context']['wrapper'] != 'i') {
              // Running the output through t() marks it as "safe" so that the markup not be sanitized.
              $content['#context']['output'] = t('<i class="' . $class . '"></i>' . $content['#context']['output']);
              $content['#context']['attributes']->removeClass($class);
            }
          }
        }
      }
    }
  }
}

@DiegoPino
Copy link
Member

@patdunlavey I like your MY_THEME_preprocess_field, you willing to make a pull against archipelago_subtheme? It feels clean and useful

@patdunlavey
Copy link
Contributor Author

Absolutely! Will do.

@patdunlavey
Copy link
Contributor Author

PR created!

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

No branches or pull requests

2 participants