Skip to content

Feat/custom icon upload#146

Open
BrutalBirdie wants to merge 2 commits intoNodeBB:mainfrom
BrutalBirdie:feat/custom-icon-upload
Open

Feat/custom icon upload#146
BrutalBirdie wants to merge 2 commits intoNodeBB:mainfrom
BrutalBirdie:feat/custom-icon-upload

Conversation

@BrutalBirdie
Copy link

This adds the option to upload a custom icon and would fix #145.

Screenshot of the edit page:

image

Screenshot of the login page:
image

@julianlam
Copy link
Member

Hi @BrutalBirdie, a couple concerns:

  1. I'm not entirely sure whether a re-implementation of upload logic is necessary. Have you look into using the methods in src/controllers/upload.js? I realize that the logic there is rather specific to NodeBB and there are hard-coded bits, but using those (and modifying them for plugin use-cases) is better worth the time than writing your own upload logic.
  2. Injecting your own CSS via the new route (+ client.js) is not necessary. The base styles can be added to a new SCSS file, and instead of a custom class, just set icons.svg in the returned strategy object. It's expecting an SVG element, but you can probably just pass in your own <i> element and set an inline-style for the background-image.

That should simplify things a bit.

@BrutalBirdie
Copy link
Author

Hello @julianlam
Thanks for the much-needed feedback.
As you might have noticed, I just wrote code that I knew would work 😅
To be 100% transparent, I have no idea how to develop a nodebb plugin.
So this PR can be viewed more like a "proof of concept".

If you, or someone, could take this PR and rewrite it into the best version suitable for nodebb, that would be awesome, and I can use that difference to learn how it should be done. :)

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

Successfully merging this pull request may close these issues.

Feature Request: Custom Icon Upload in addition to FontAwesome Icon

2 participants

Comments