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.

Sign up for GitHub

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

Jump to bottom

navigation/dashboard.phtml: Use ipl-web's Icon for fa- icons #5278

Merged
merged 2 commits into from
Nov 4, 2024
Merged

navigation/dashboard.phtml: Use ipl-web's Icon for fa- icons #5278

merged 2 commits into from
Nov 4, 2024

Conversation

nilmerg
Copy link
Member

@nilmerg nilmerg commented Oct 7, 2024

resolves #5277

@nilmerg nilmerg added enhancement New feature or improvement area/framework Affects third party integration/development area/ui Affects the user interface labels Oct 7, 2024
@nilmerg nilmerg added this to the 2.12.2 milestone Oct 7, 2024
@nilmerg nilmerg requested a review from raviks789 October 7, 2024 14:26
@nilmerg nilmerg self-assigned this Oct 7, 2024
@cla-bot cla-bot bot added the cla/signed label Oct 7, 2024
@nilmerg
Copy link
Member Author

nilmerg commented Oct 7, 2024 •
edited
Loading

requires Icinga/ipl-web#232
without it, the result is the same as without this change

@moreamazingnick
Copy link
Contributor

since the branch is called fa-icons for everyone...
does this fit into that?
#5205

@nilmerg
Copy link
Member Author

nilmerg commented Oct 8, 2024

In theory, for menu items, yes. Patching url.php makes not much sense to me, it's legacy code and links with fontawesome icons are constructed differently nowadays.

raviks789
raviks789 previously approved these changes Oct 8, 2024
View reviewed changes
@nilmerg
Copy link
Member Author

nilmerg commented Oct 10, 2024

@moreamazingnick Now it does fit.
@raviks789 I've added the map as discussed recently.

@nilmerg nilmerg linked an issue Oct 10, 2024 that may be closed by this pull request
Allow fontawsome icons as menu items #5205
Closed
@nilmerg
navigation/dashboard.phtml: Use ipl-web's Icon for fa- icons
526de93
@nilmerg
url.php: Add partial support for fontawesome
e34c174 
Partial because the helper method is not the preferred way
anymore to create an icon. So I simplified the detection
to only check whether the given icon is a legacy one, as
those are smaller in number. Though, this leads to some fa
icons being identified as legacy, as the names equal. But,
it's the legacy helper after all... Anyone wanting to make
sure to get fontawesome icons, must add the `fa-` prefix.
@nilmerg nilmerg force-pushed the feature/fa-icons-for-everyone-5277 branch from f599035 to e34c174 Compare November 4, 2024 09:26
@nilmerg nilmerg merged commit e12574f into main Nov 4, 2024
22 checks passed
@nilmerg nilmerg deleted the feature/fa-icons-for-everyone-5277 branch November 4, 2024 09:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@raviks789 raviks789 raviks789 approved these changes

Assignees

@nilmerg nilmerg

Labels
area/framework Affects third party integration/development area/ui Affects the user interface cla/signed enhancement New feature or improvement
Projects
None yet
Milestone
2.12.2
3 participants
@nilmerg @moreamazingnick @raviks789