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

Improve site label/icon positioning #66

Merged

Conversation

JoryHogeveen
Copy link
Contributor

For site labels I've converted the inline CSS to a selector.
Much easier for other plugins to use the hook and simply apply the same styles through the selector.

Also tweaked the favicon position based on the default WP icon.

Much easier for other plugins to use the hook and simply apply the same styles through the selector
@bueltge
Copy link
Owner

bueltge commented Oct 1, 2020

Hi @JoryHogeveen
Thanks a lot for the improvement!
However should we not remove the function that returns only static CSS code? I mean we should add this CSS string to the hook as default - multisite_enhancements_add_admin_bar_favicon_css. If users will change them she uses the hook and changes our default. The function is not necessary, Only a return of a string and usage of variable makes ore slowly ind not better to read the code. Also, the hook inside the constructor makes it not easy if we would test them, like via PhpUnit.

@JoryHogeveen
Copy link
Contributor Author

Hi @bueltge
I've moved the hooks outside of the constructor method.
Hoever, I'm not sure I understand what you mean with the filter. Isn't the whole idea of a filter to be able to change the default behavior?
I initially wanted to modify the multisite_enhancements_add_admin_bar_favicon hooks but ended up leaving that as it is for backwards compatibility. However, since that hook also passes the <style> tags I'd say that it might be better to deprecate that one.

@JoryHogeveen
Copy link
Contributor Author

Hi @bueltge
Please let me know and I will update the PR accordingly!

@bueltge bueltge merged commit 239654c into bueltge:master Nov 9, 2020
@JoryHogeveen JoryHogeveen deleted the 20-10-01-sitelist-icon-positioning branch November 9, 2020 12:04
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