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

Add offical logos for Python and R #1452

Merged
merged 5 commits into from
Mar 22, 2021
Merged

Conversation

ajbozarth
Copy link
Member

Currently we use the jupyterlab python logo, but with the addition
of R support we would like to have pipelines use the the offical
logos for each node instead of ones designed for the lab file browser

Fixes #1445

Developer's Certificate of Origin 1.1

   By making a contribution to this project, I certify that:

   (a) The contribution was created in whole or in part by me and I
       have the right to submit it under the Apache License 2.0; or

   (b) The contribution is based upon previous work that, to the best
       of my knowledge, is covered under an appropriate open source
       license and I have the right under that license to submit that
       work with modifications, whether created in whole or in part
       by me, under the same open source license (unless I am
       permitted to submit under a different license), as indicated
       in the file; or

   (c) The contribution was provided directly to me by some other
       person who certified (a), (b) or (c) and I have not modified
       it.

   (d) I understand and agree that this project and the contribution
       are public and that a record of the contribution (including all
       personal information I submit with it, including my sign-off) is
       maintained indefinitely and may be redistributed consistent with
       this project or the open source license(s) involved.

Currently we use the jupyterlab python logo, but with the addition
of R support we would like to have pipelines use the the offical
logos for each node instead of ones designed for the lab file browser

Fixes elyra-ai#1445
@ajbozarth ajbozarth added the status:Work in Progress Development in progress. A PR tagged with this label is not review ready unless stated otherwise. label Mar 17, 2021
@ajbozarth ajbozarth added this to the 2.2.0 milestone Mar 17, 2021
@ajbozarth ajbozarth requested a review from lresende March 17, 2021 21:57
@ajbozarth ajbozarth self-assigned this Mar 17, 2021
@elyra-bot
Copy link

elyra-bot bot commented Mar 17, 2021

Thanks for making a pull request to Elyra!

To try out this branch on binder, follow this link: Binder

@ajbozarth
Copy link
Member Author

Leaving this WIP for a few reasons

  1. need to test/update it once Enable support for adding R Script to Pipeline #1418 is merged
  2. I'm not sure yet on if I've changed the logo everywhere it should be changed

Places to change the python logo and whether I've changed them:

  • In the file browser. I have NOT changed this, but am unsure if we should
  • In the launcher. I have changed this, but am unsure if we should
  • In the tab bar for python editor. I have NOT changed this, but it should match the file browser no matter our choice.
  • In pipelines. I changed this and it should use the new icon.

@bourdakos1
Copy link
Member

I think we should leave the file browser and tab bar as is and have the pipeline nodes match that.

For the launcher icons, I think we should do our best to match the other present launcher icons. I updated the "official" svg to match the other pngs more closely: https://codesandbox.io/s/python-logo-z05p4?file=/src/App.tsx

I personally think the icon in this PR is awkwardly too close to the other launcher icons, but not close enough so it draws your eye in a bad way. If we don't use something closer to the pngs, then I think we should go back to using the icon from file browser

@ajbozarth
Copy link
Member Author

I've pushed a few updates:

Some screenshots of the current state:
Screen Shot 2021-03-18 at 1 46 35 PM
Screen Shot 2021-03-18 at 1 46 58 PM
Screen Shot 2021-03-18 at 1 47 15 PM

@ajbozarth
Copy link
Member Author

The upstream PR has been merged and I've pushed an update here to support both python and r icon in the side bar, new screenshots:

Screen Shot 2021-03-22 at 1 22 19 PM

Screen Shot 2021-03-22 at 1 22 51 PM

Screen Shot 2021-03-22 at 1 22 58 PM

@ajbozarth ajbozarth removed the status:Work in Progress Development in progress. A PR tagged with this label is not review ready unless stated otherwise. label Mar 22, 2021
@lresende
Copy link
Member

The related lab PR jupyterlab/jupyterlab#9958 is now available on JupyterLab 3.0.12. Also, on existing pipelines, as the node icon is part of the pipeline, you would need to re-add the node to the pipeline.

@ajbozarth
Copy link
Member Author

Also, on existing pipelines, as the node icon is part of the pipeline, you would need to re-add the node to the pipeline.

It may be worth including updating the icon as a pipeline version update

@lresende
Copy link
Member

LGTM:

image

@lresende lresende merged commit 835ac89 into elyra-ai:master Mar 22, 2021
@ajbozarth ajbozarth deleted the logos branch March 22, 2021 22:30
@ajbozarth ajbozarth mentioned this pull request Mar 22, 2021
15 tasks
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.

Update icons used for Python and R
3 participants