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

Ability to customize the FA icon class #54

Open
sburkett opened this issue Dec 23, 2020 · 5 comments
Open

Ability to customize the FA icon class #54

sburkett opened this issue Dec 23, 2020 · 5 comments

Comments

@sburkett
Copy link

sburkett commented Dec 23, 2020

Currently, I use FontAwesome Pro's duotone icons (class fad). Unfortunately, the icon classes used in this extension (fad fa-address-book) are currently hardcoded in vendor/fof/user-directory/js/src/forum/index.js. Would love the ability to customize this the way other extensions do.

Just a suggestion. Thanks!

@clarkwinkelmann
Copy link
Member

Actually, very few extensions allow customizing FA icons for fixed contexts (like navigation or badge icons)

The only two extensions I know that do that are Who Read and Byobu.

I'm personally not a fan of this solution, because it creates additional database load for each request for something that could be achieved with cached custom CSS 😬

If you just want to customize it on your forum, you can do it with custom CSS. Solutions like these should work https://discuss.flarum.org/d/7078-change-tag-left-sidebar-icon-with-custom-font-awesome-icon/15

In case we do want to implement customizable icon in this extension, here's the link to other implementations:

Reference for Who Read implementation
https://github.com/clarkwinkelmann/flarum-ext-who-read/blob/caab29f2a6727ff3bd382f4b5913cb312202dc98/js/src/admin/index.js#L65-L72
https://github.com/clarkwinkelmann/flarum-ext-who-read/blob/caab29f2a6727ff3bd382f4b5913cb312202dc98/src/ForumAttributes.php#L29
https://github.com/clarkwinkelmann/flarum-ext-who-read/blob/40cca20d7573777f52f425380e30a640c1d86a62/js/src/forum/utils/appendReaderBadges.js#L29-L34

Reference for Byobu implementation
https://github.com/FriendsOfFlarum/byobu/blob/2222e0f419ae2b73be09c2e68fe24bafb5b1a1a3/js/src/admin/components/ByobuSettingsPage.js#L24-L29
https://github.com/FriendsOfFlarum/byobu/blob/0f829971da1d6a34189a849f08eb8c5c8cc269f4/extend.php#L160-L166
https://github.com/FriendsOfFlarum/byobu/blob/2222e0f419ae2b73be09c2e68fe24bafb5b1a1a3/js/src/forum/extend/User.js#L28

@sburkett
Copy link
Author

sburkett commented Jan 5, 2021

I guess I'm confused. If the icon setting were stored in the Setting model along with all the other settings, why would this require an additional database load per request? Example from Byobu:

image

Granted, I'm not overly familiar (yet) with the Flarum architecture, but aren't all of these settings loaded up front (and/or cached)? Or are you saying every extension has to make a separate query just to retrieve their settings on each request? That would seem strange to me from a design perspective.

@clarkwinkelmann
Copy link
Member

Yes my performance observation is based on the fact that in Flarum, by default, every setting is retrieved with a separate database request. This should be optimized at the Flarum level at some point 😅

@sburkett
Copy link
Author

sburkett commented Jan 6, 2021

😮 😨

@sburkett
Copy link
Author

sburkett commented Jan 6, 2021

Yeah, I just took a quick look. While I haven't tested this, it would appear that core/src/Settings/DatabaseSettingsRepository.php doesn't cache, whereas the MemorySettingsRepository does.

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