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

Provide an option to display the sun or moon icon in solid #73

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Yippy
Copy link

@Yippy Yippy commented Apr 11, 2024

Added a new option for Admin's the ability to change the icon between solid and outline, in my opinion the sun outline looks like a cog. Also for consistency for the website theme, many uses solid icons.

Changes proposed in this pull request:
This effects the extension setting with the added new option.
Header and dropdown option icon, through-out the website.

Reviewers should focus on:
Top right corner icons

Screenshot
image

Confirmed

  • Frontend changes: tested on a local Flarum installation.
  • Backend changes: tests are green (run composer test).

@Yippy
Copy link
Author

Yippy commented Apr 12, 2024

Noticed an old issue mentioning about the sun looking like a gear icon, though this PR is defaulted for the regular style with the option to change to solid. I'm not sure how to do a composer test, but the PR is ready for review.
Fixes: #40

@imorland
Copy link
Member

Hey! Thanks for the PR!

I actually think that we should just switch over to using the solid icons here anyway, as these would then match the default icons used across Flarum.

Would you mind making that change for me?

PS, please don't commit the dist files, these will be built automatically by the bot once merged :)

@Yippy
Copy link
Author

Yippy commented May 22, 2024

Hey! Thanks for the PR!

I actually think that we should just switch over to using the solid icons here anyway, as these would then match the default icons used across Flarum.

Would you mind making that change for me?

PS, please don't commit the dist files, these will be built automatically by the bot once merged :)

But wouldn't this effect everyone with this package, as I'm not sure if it's too late and people got use to the outline. Yes the original default theme only use solid icons. Do you want me to:

  1. Remove the toggle and just assume everyone want solid icon
  2. Keep the toggle, but default to solid icon. Providing the people the option to change to outline?

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.

None yet

2 participants