FriendsOfFlarum / user-directory

The permission based public user directory extension for your Flarum forum.
https://discuss.flarum.org/d/5682
MIT License
22 stars 20 forks source link

Ability to customize the FA icon class #54

Open sburkett opened 3 years ago

sburkett commented 3 years ago

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 commented 3 years ago

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 :grimacing:

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 commented 3 years ago

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 commented 3 years ago

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 :sweat_smile:

sburkett commented 3 years ago

😮 😨

sburkett commented 3 years ago

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.