Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

FEATURE: Add support for native emojis + Discourse emojis + use category logo as icon #17

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

carsick
Copy link

@carsick carsick commented Feb 6, 2023

@carsick carsick requested a review from pmusaraj February 6, 2023 05:41
Copy link
Contributor

@pmusaraj pmusaraj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apologies for the delay @carsick!

This is impressive and it expands the functionality ofthe component considerably. In fact, my feeling is that it expands it a bit too much.

What do you think about reducing scope to just adding support for native + Discourse emojis?

@@ -2,7 +2,7 @@ category_icon_list:
default: "help,question-circle,#CC0000,partial|"
type: "list"
list_type: "simple"
description: 'Enter comma-delimited configuration for categories, in the format "slug,icon,color,match". Colour in format #123456 or "categoryColor" to use the default color for the category (same as the Badge color). If match is "partial" then the slug need only partially match the category-slug, otherwise an exact match is required'
description: ' Enter comma-delimited configuration for categories<br><br>Format: "slug,icon,color,partial"<br><br>slug — The category slug.<br><br>icon — Can be either (1) a Font Awesome icon, (2) a singular unicode character "😀", or (3) a forum emoji formatted like ":grinning:"<br><br>color (optional) — Color for the icon. Use a hex code with the "#" character. Default color is the category color.<br><br>partial (optional) — A color must be set first to activate this. Write "partial" if the slug only needs to partially match the category-slug, otherwise an exact match is required. Useful for subcategories with identical names.'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Default color is the category color.

I don't think this is a bad idea per se, but it is a breaking change. For existing users of the component, it will result in a change in behaviour. Except for sites that were previosuly using categoryColor.

Is there a strong reason to make this change? If not, I think we should keep the functionality as is.

settings.yml Outdated
use_category_logo:
type: bool
default: false
description: "Use the category's uploaded logo as its icon.<br><br><strong>Overrides</strong> Font Awesome icons and emojis.<br><br>Category logos can be uploaded through individual category settings."
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels more like a separate theme component because checking this theme setting essentially disables all the other ones. I could see this as either a separate theme component or, maybe, as an option in core's category settings.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants