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

Some icons are not translated properly #17

Open
amotl opened this issue Jan 17, 2019 · 9 comments
Open

Some icons are not translated properly #17

amotl opened this issue Jan 17, 2019 · 9 comments
Labels

Comments

@amotl
Copy link

amotl commented Jan 17, 2019

Introduction

We just tried to get your recent amendments from #3, #13 and #14 on board but obviously this wouldn't help as the Grunt task has not been run. No hurries on that one, we are already very happy with the current state and will just continue reporting things we come across - even minor ones you might have recognized already - and hope you are okay with that.

Preface

We just ran a full Discourse rebuild from the command line and recognized a subtle regression barely able to spot between the version running before, which was installed a few days ago and the current version v2.2.0.beta8 +37.

Problem

The fa-eye icon designated to signal the state of a topic item being listed or unlisted in the topic list on the right hand side just disappeared.

With Default theme

image

With Material theme

image

@amotl
Copy link
Author

amotl commented Jan 17, 2019

Investigation

The HTML code currently rendered is

<i class="fa fa-far-eye-slash d-icon d-icon-far-eye-slash unlisted"></i>

Seeing the new FA naming scheme slip through there, this might be coming from the Font Awesome transition, however we haven't been able to spot any specific commit in Discourse coming in recently.

Workaround

Amending this to

<i class="fa fa-eye-slash d-icon d-icon-fa-eye-slash unlisted"></i>

made the icon appear again.

This snippet of code added to the end of the function faClasses applies this specific amendment to all icons in question.

// Fix MBT#17: Hack to translate class names like "fa-far-eye-slash" to "fa-eye-slash" again
classNames = classNames.replace('far-', 'fa-').replace('fa-fa-', 'fa-');

Hope this helps.

@sesemaya
Copy link
Collaborator

This is very likely related to Discourse switching to Font Awesome 5 SVG icons.

There is a workaround for this in common/head_tag.html and it may need to be updated.

We'll try to get our Discourse updated and then have a look at this issue.

@sesemaya sesemaya added the bug label Jan 23, 2019
@amotl
Copy link
Author

amotl commented Apr 4, 2019

After the most recent update, the "edit"- and "delete"-post icons disappeared. To mitigate this, we added

// Fix MBT#17: Translate "-alt" icons to non-"-alt"
classNames = classNames.replace('-alt', '');

at the end of the Font Awesome icons support function "faClasses".

We also found that fa fa-clock (used for "Set Topic Timer") is missing completely.

@ryanmstokes
Copy link

ryanmstokes commented Apr 8, 2019

How can I fix this please? Ive tried to add this line to the function in the head file but it makes no difference.

@amotl
Copy link
Author

amotl commented Apr 9, 2019

Dear Maya,

  1. We can confirm the fa-eye icon works with the most recent version:
    image

  2. The "edit"- and "delete"-post icons are also working flawlessly:
    image

  3. The fa-clock icon used for "Set Topic Timer" is still missing:
    image

However, this is really a minor issue, so this is just for the records.

Thanks again for your awesome work on this theme!

With kind regards,
Andreas.

@amotl amotl changed the title Some icons not translated properly due to recent upstream change Some icons are not properly translated Apr 9, 2019
@amotl amotl changed the title Some icons are not properly translated Some icons are not translated properly Apr 9, 2019
@ryanmstokes
Copy link

ryanmstokes commented Apr 10, 2019

It is still broken for me, the edit and delete icons are missing, the checkbox icons on surveys are missing, how do I fix this please?

@amotl
Copy link
Author

amotl commented Apr 10, 2019

Dear @ryanmstokes,

you might want to apply the workaround as outlined above for a quick fix. For doing so, you will find the function faClasses at https://discourse.example.org/admin/customize/themes/22/common/head_tag/edit when replacing the URL parts appropriately with the corresponding ones of your installation or just navigating to "Admin » Customize » Themes » Daemonite Material » Edit CSS/HTML » Common » </head>".

There, you might want to insert classNames = classNames.replace('-alt', ''); right before return classNames;, like that:

  // Support for Font Awesome icons
  function faClasses(icon, params) {
    let classNames = `fa fa-${icon.replacementId || icon.id} d-icon d-icon-${
      icon.id
    }`;
  
    if (params) {
      if (params.modifier) {
        classNames += ' fa-' + params.modifier;
      }

      if (params['class']) {
        classNames += ' ' + params['class'];
      }
    }

    // Fix MBT#17: Translate "-alt" icons to non-"-alt" ones to
    // counter disappearing "edit"- and "delete"-post icons.
    // https://github.com/Daemonite/discourse-material-theme/issues/17#issuecomment-479716834
    classNames = classNames.replace('-alt', '');

    return classNames;
  }

Please also share the Discourse version you are running on with us and check whether you updated the theme to the most recent version.

Asking that, I would like to mention that it already works perfectly for us without that workaround after the last upgrade. Now, we are running Discourse v2.3.0.beta6 +148, probably with the most recent version of discourse-material-theme.

With kind regards,
Andreas.

@svrcore
Copy link

svrcore commented Apr 11, 2019

I have this issue too, my version is 2.2.4.

image

@amotl
Copy link
Author

amotl commented May 8, 2019

Dear @svrcore, did this issue resolve for you? Otherwise, you might want to try the hotfix as outlined above which worked around the issue on our installation the other day.

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

No branches or pull requests

4 participants