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: add md- prefix to regular ion icons #265

Closed
wants to merge 3 commits into from

Conversation

UziTech
Copy link
Member

@UziTech UziTech commented Oct 2, 2019

prefixes ion icon with md- when no prefix is specified

fixes #264

Copy link
Contributor

@ericcornelissen ericcornelissen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Though not ideal, I think this is the easiest solution to give some backwards compatibility w.r.t. #264. Most notably this will not cover the icons that are now prefixed with logo- (as they are not available with either ios- or md-). That said, this looks good to me and should probably be merged and released as soon as possible.


I think choosing the "refresh" icon for the new test is appropriate as it is unlikely to be removed in the future

@VivaldoMendes
Copy link

I updated to 1.1.13 and some of my icons are working, while others are not (Start Remote Julia Process, Interrupt Julia,, Stop Julia and Run All). I am afraid of trying to do something that instead of correcting the existing problems, will create new ones (Atom is still working for me). Do you have any idea whether a new version will come out soon with these problems solved? Thank you.

@suda
Copy link
Collaborator

suda commented Oct 3, 2019

Like Eric said, defaulting to md- prefix would break the logo icons and the users might want to use the ios- prefix. I'll close this PR in favor of releasing tool-bar 1.2.0 with information about the changes. Thanks for doing this Tony!

@suda suda closed this Oct 3, 2019
@UziTech
Copy link
Member Author

UziTech commented Oct 3, 2019

@VivaldoMendes the best solution seems to be changing the icon names to match the new version of ionicons prefixed with md- or logo-

@VivaldoMendes
Copy link

VivaldoMendes commented Oct 3, 2019 via email

@UziTech
Copy link
Member Author

UziTech commented Oct 3, 2019

If there are any packages that supply the buttons for tool-bar then those packages will need to update their code

@UziTech UziTech deleted the patch-1 branch October 3, 2019 23:32
@ericcornelissen
Copy link
Contributor

I convinced 33 master students to install Juno and Julia in their computers and give up Matlab. It was not easy, and with this kind of regular instability in Atom will not help much.

First off, this is not an instability in Atom but with a particular package in Atom.

I am relatively new to Atom and Julia. Where am I supposed to produce the changes? I tried tool-bar 1.2.0 + settings, but apparently it is not here.

Then I tried Project+tool-bar+iconsets+ionicons. But here what should I do? I opened the file ionicons.css. There is a huge amount of lines with md- and logo. Which ones should I choose in order to get back the buttons for:

Start Remote Julia Process
Interrupt Julia
Stop Julia
Run All

I don't think ionicons ever had icons for Julia, more likely you used specific icons from ionicons (like md-play-circle) for those Julia action. Can you share your toolbar configuration @VivaldoMendes, or where you got your configuration from? That would make it easier to help you find the icons you need.

@VivaldoMendes
Copy link

VivaldoMendes commented Oct 4, 2019 via email

@UziTech
Copy link
Member Author

UziTech commented Oct 4, 2019

@VivaldoMendes looks like the julia client will get an update soon that should fix it

JunoLab/atom-julia-client#626

@VivaldoMendes
Copy link

VivaldoMendes commented Oct 4, 2019 via email

@ericcornelissen
Copy link
Contributor

@VivaldoMendes I'm sorry your experience with regards to Atom package thus far hasn't been great. I definitely learned my lesson with regards to the fact that my Pull Request - the one that caused this issue - caused issues in other packages. And I will do my best to consider up-stream dependencies (i.e. the package that you use which use this project) in the future.

I guess a somewhat unfortunate side-effect of open source is that sometimes things go wrong because there isn't an entire software team behind the product you're using.

@VivaldoMendes
Copy link

VivaldoMendes commented Oct 4, 2019 via email

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.

icons disappear after update to v.1.1.13
4 participants