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

replacing the icons #67

Open
brianteeman opened this issue Mar 30, 2024 · 13 comments
Open

replacing the icons #67

brianteeman opened this issue Mar 30, 2024 · 13 comments

Comments

@brianteeman
Copy link
Contributor

Unfortunately it is not as simple as replacing the content attribute with the CSS variables currently being used as the built-in css also manipulates the icons eg

accessibility/src/main.ts

Lines 652 to 657 in 6077514

._access-menu ul li button[data-access-action="increaseTextSpacing"]:before {
content: var(--_access-menu-item-icon-increase-text-spacing, ${!this.options.icon.useEmojis ? '"unfold_more"' : '"🔼"'});
transform: rotate(90deg) translate(-7px, 2px);
top: 14px;
left: 0;
}

@ranbuch
Copy link
Owner

ranbuch commented Mar 30, 2024

@brianteeman Version 5.0.4 will support more CSS variables in order to adjust the icons better.
Trust me, you don't want to write your own CSS. Even if you'll manage to do so your code might break in future releases.

Keep me posted.

@brianteeman
Copy link
Contributor Author

Switching from material icons to my own icon set lets me see various other places where content variables are needed. I will try to list them all here

  text: "".concat(!this.options.icon.useEmojis ? 'close' : 'X')
  text: "".concat(!this.options.icon.useEmojis ? 'refresh' : '♲')
  content: ${!this._acc.options.icon.useEmojis ? '"mic"' : '"🎤"'}

and finally
'class': this._acc.options.icon.useEmojis ? '' : 'material-icons',
which I guess really should be using the variable fontClass

@brianteeman
Copy link
Contributor Author

i am guessing that similar changes are also required for the main icon as that is also hardcoded for either material or emoji

@ranbuch
Copy link
Owner

ranbuch commented Jun 15, 2024

@brianteeman Please try version 5.1.4 and let me know.

@brianteeman
Copy link
Contributor Author

brianteeman commented Jun 16, 2024

unless I am missing something this doesnt work. With this PR the code generated is
<button class="_menu-close-btn _menu-btn iconfont" title="Hotkey: ctrl+alt+a" tabindex="0">close</button>

the problem is that while material will replace the close with an icon fotnaweseome and others will not as they rely on a class

@ranbuch
Copy link
Owner

ranbuch commented Jun 17, 2024

@brianteeman The readme file has this example (I should add it to the site as well):

const options = {
    icon: {
        fontFaceSrc: ['https://cdnjs.cloudflare.com/ajax/libs/font-awesome/6.5.1/css/v4-font-face.min.css'],
        fontFamily: '"FontAwesome"',
        img: '[optional - URL for an image that will replace the menu icon]',
        closeIcon: '[optional - replacement text for the close menu icon]',
        resetIcon: '[optional - replacement text for the reset all icon]'
    }
};
new Accessibility(options);

closeIcon and resetIcon has been added according to your previous comments.

@brianteeman
Copy link
Contributor Author

Yes I saw that but closeIcon/resetIcon does not work with fontawesome. did you try it?

@ranbuch
Copy link
Owner

ranbuch commented Jun 17, 2024

I see now that it's only working for Emoji mode. Please try version 5.1.5.

@brianteeman
Copy link
Contributor Author

that doesnt work either - did you test it yourself? the problem is the same as I already said

the problem is that while material will replace the close with an icon fontaweseome and others will not as they rely on a class

@ranbuch
Copy link
Owner

ranbuch commented Jun 19, 2024

Please try version 6.0.1 and see updated documentation. It's a bit more complicated because we need to change the HTML itself sometimes, not just CSS variables.

@brianteeman
Copy link
Contributor Author

thanks - will test later today and report back but at a quick glance it does appear to be on the right path

@kvonspiczak
Copy link

kvonspiczak commented Sep 26, 2024

I'm trying to replace the icons with a local fontface, since we already have material icons loaded anyways.
It seems to be working fine, however we're still getting an error in the developer console regarding a CSP violation:

Refused to load the stylesheet 'https://fonts.googleapis.com/icon?family=Material+Icons' because it violates the following Content Security Policy directive: "style-src-elem 'self' 'unsafe-inline' 'report-sample'".

Am I missing something here, or why is the tool still trying to load the icons from google?
I did it like in the documentation:

const options = {
    icon: {
        fontFaceSrc: ['https://my-domain.com/_assets/some-random-hash/StyleSheets/main.min.css']
    }
};

@ranbuch
Copy link
Owner

ranbuch commented Sep 28, 2024

Hi @kvonspiczak
Your options object looks good. How / where can I reproduce the issue? Are you sure the injection is related to this library?

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

3 participants