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

Che plugin icons degrade under light theme - should just use icon (no wordmark) #15436

Closed
1 of 3 tasks
nickboldt opened this issue Dec 9, 2019 · 8 comments
Closed
1 of 3 tasks
Labels
area/plugin-registry kind/bug Outline of a bug - must adhere to the bug report template. severity/P1 Has a major impact to usage or development of the system.

Comments

@nickboldt
Copy link
Contributor

nickboldt commented Dec 9, 2019

Describe the bug

Under the dark theme, all the Che icons say "Eclipse Che" when you open the Plugins view from CTRL-SHIFT-J or View > Plugins.

image

But on the light theme, the text nearly disappears (white text on light grey background), leaving a big space between the icon and the content.

image

Perhaps we should remove the text from the icon used in Che's plugin view panel?

Che version

  • latest
  • nightly
  • other: 7.3.2, 7.3.3

Steps to reproduce

Start a workspace. Switch to light theme. CTRl_SHIFT-J or View > Plugins to toggle the plugins view open/closed. Note icons.

Expected behavior

Should either have a wordmark that works on light theme, or JUST the Che icon (no text).

Removing text is simpler -- the text is not needed, since everyone using Che has seen its icon so the need to co-brand the icon w/ the wordmark should be non-existent.

@nickboldt nickboldt added the kind/bug Outline of a bug - must adhere to the bug report template. label Dec 9, 2019
@nickboldt nickboldt changed the title Che plugin icons degrade under light theme Che plugin icons degrade under light theme - should just use icon (no wordmark) Dec 9, 2019
@che-bot che-bot added the status/need-triage An issue that needs to be prioritized by the curator responsible for the triage. See https://github. label Dec 9, 2019
@ibuziuk ibuziuk added severity/P2 Has a minor but important impact to the usage or development of the system. severity/P1 Has a major impact to usage or development of the system. team/ide2 and removed severity/P2 Has a minor but important impact to the usage or development of the system. status/need-triage An issue that needs to be prioritized by the curator responsible for the triage. See https://github. labels Dec 9, 2019
@ibuziuk
Copy link
Member

ibuziuk commented Dec 9, 2019

Adding P1 label since this affects CRW

@amisevsk
Copy link
Contributor

amisevsk commented Dec 9, 2019

Related: #14510

@ericwill
Copy link
Contributor

Chopping off the text would work, but does it really make sense (from a UX perspective) to show the same logo for every plugin?

I like the approach from #14510 because I think it makes more sense to store the icon of each plugin locally within the registry. Maybe have an images folder in every plugin folder that contains the icon? There was a concern about licensing of the images but if we are pulling them from the upstream extension's GitHub I don't think it's an issue.

@amisevsk
Copy link
Contributor

@ericwill I agree that #14510 is the way to go eventually, but it also depends on being able to resolve the relative icon URLs -- if the plugin registry serves e.g. /icons/jdt.svg, Theia will need to make that reference absolute before showing it to the user.

This issue is somewhat separate, as we currently default to using the same svg everywhere: https://www.eclipse.org/che/images/logo-eclipseche.svg. Fixing this issue would be as simple finding a plain Che logo hosted somewhere statically and replacing the links in the plugin registry.

@ericwill
Copy link
Contributor

In that case I suggest we merge the existing PR for #14510 and leave the question of icon serving/storing for another day. At least this way we get some interim benefit of not having the same icon for every plugin in the list.

@nickboldt
Copy link
Contributor Author

This no longer affects downstream directly as we've removed the Che icons from CRW's plugin view.

https://issues.redhat.com/browse/CRW-561

So... if everyone is cool with the ugliness in the light theme as it stands today in Che, then we can close this in favour of #14510

In that case I suggest we merge the existing PR for #14510 and leave the question of icon serving/storing for another day.

I don't see a PR linked from that issue. Am I blind?

@ericwill
Copy link
Contributor

So... if everyone is cool with the ugliness in the light theme as it stands today in Che, then we can close this in favour of #14510

In that case I suggest we merge the existing PR for #14510 and leave the question of icon serving/storing for another day.

I don't see a PR linked from that issue. Am I blind?

There is one but it's closed. I should have been more clear, I meant adopting an approach similar to that PR.

I am fine with closing this issue if we are going to go with #14510.

@ericwill
Copy link
Contributor

Closing in favour of #14510

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/plugin-registry kind/bug Outline of a bug - must adhere to the bug report template. severity/P1 Has a major impact to usage or development of the system.
Projects
None yet
Development

No branches or pull requests

6 participants