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

Fix custom logo #197

Closed
wants to merge 4 commits into from
Closed

Fix custom logo #197

wants to merge 4 commits into from

Conversation

kodypeterson
Copy link
Contributor

Custom logos are not loading as /-/static/-/logo-sm is not the endpoint it is /-/logo-sm

Custom logos are not loading as /-/static/-/logo-sm is not the endpoint it is /-/logo-sm
By adding the second background-image the custom one gets overwritten. I know you were trying to save a network call, but that wont work.
@kodypeterson
Copy link
Contributor Author

I will fix the tests....

@rlidwka
Copy link
Owner

rlidwka commented Feb 1, 2015

Custom logo works fine for me. But it's probably because of data-uri. Which browser do you use? Does it support background-image: url(data:image/png ...)?

The fix isn't entirely correct, because people can override /. I mean, you can set up nginx to serve sinopia under https://example.org/sinopia/, in which case logo uri should be https://example.org/sinopia/-/logo-sm, not https://example.org/-/logo-sm like it will be with this fix.

Maybe replacing it with ../../-/logo-sm would be better.

I will fix the tests....

I believe tests aren't related to the PR, so don't bother. They probably changed something on Travis again.

@kodypeterson
Copy link
Contributor Author

So, what if I were to add a baseURL in the config...

Also, this resolves setting a custom logo via the config. That was not working as the data-uri over-rode my custom logo.

@kodypeterson
Copy link
Contributor Author

So, re-reading your comment I guess we have a different idea of what custom logo is.

So, you have provided a way in the config for a user do define a location to a logo. This si the functionality that is not working. It always shows your npm logo. I had found it was due to the data-uri always overriding the background-image: url( /-/logo-sm );

Does this make more sense?

@rlidwka
Copy link
Owner

rlidwka commented Mar 8, 2015

Okay, I see what you mean. I don't really know why custom logo was added, it came from a pull request 1.5 years ago.

Question: does someone really need a custom logo?

Maybe remove that option entirely, and let people who want to customize interface to supply a custom template file like it was suggested in #208?

@kodypeterson
Copy link
Contributor Author

Yeah either or. I know that our use case wants the support for custom logo.

@rlidwka
Copy link
Owner

rlidwka commented Mar 29, 2015

Merged in bf40ceb with a bunch of related fixes in d7c95d6 (since /-/logo is unused, I changed /-/logo-sm to /-/logo to avoid having dead code).

Thanks, and sorry for the delay.

@rlidwka rlidwka closed this Mar 29, 2015
@kodypeterson
Copy link
Contributor Author

@rlidwka Cool, Thanks!

rmg pushed a commit to strongloop-forks/sinopia that referenced this pull request Jun 1, 2017
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.

2 participants